Remove requirments-elyra.txt from the code base #761

dibryant · 2024-11-04T14:18:14Z

Fixes for https://issues.redhat.com/browse/RHOAIENG-11068

Description

Remove requirments-elyra.txt from the code base

How Has This Been Tested?

@jiridanek:

ran make tests for images
pip install -r from the modified requirement files

Merge criteria:

The commits are squashed in a cohesive manner and have meaningful messages.
Testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious).
The developer has manually tested the changes and verified that the changes work

jiridanek · 2024-11-05T15:11:47Z

runtimes/datascience/ubi9-python-3.11/kustomize/base/requirements-elyra.txt

I thought we decided to keep these files present, just delete the content. Has anything changed?

Empty out "*/utils/requirements-elyra.txt", with comments to keep it for sanity
remove all "/kustomize/base/requirements-elyra.txt"

hmm

Ahhh I see will revert it back! Sorry about that

Empty out "*/utils/requirements-elyra.txt", with comments to keep it for sanity
remove all "/kustomize/base/requirements-elyra.txt"

This is old summary. Last time we discussed on the scrum to keep the /utils/requirements-elyra.txt and put its content in comments, because otherwise the bootstraper.py will fail.

Makefile

harshad16

Great initiative, shared some comments

harshad16 · 2024-11-15T01:45:00Z

Makefile

-			$(KUBECTL_BIN) exec runtime-pod -- /bin/sh -c "python3 -m pip install -r /opt/app-root/elyra/requirements-elyra.txt && \
-				curl https://raw.githubusercontent.com/nteract/papermill/main/papermill/tests/notebooks/simple_execute.ipynb --output simple_execute.ipynb && \
-				python3 -m papermill simple_execute.ipynb output.ipynb > /dev/null" ; \
+			$(KUBECTL_BIN) exec runtime-pod -- /bin/sh -c "python3 -m pip install pipenv --three sync && \


$(KUBECTL_BIN) exec runtime-pod -- /bin/sh -c "curl https://raw.githubusercontent.com/opendatahub-io/elyra/refs/heads/main/etc/generic/requirements-elyra.txt --output req.txt && \ python3 -m pip install -r req.txt && \ curl https://raw.githubusercontent.com/nteract/papermill/main/papermill/tests/notebooks/simple_execute.ipynb --output simple_execute.ipynb && \ python3 -m papermill simple_execute.ipynb output.ipynb > /dev/null" ; \

Try this.
As, the reason for using the requirements.txt is so that , we can make sure that packages are installable ,
perhaps, we can extend this in further tasks, to do some extra checks.

runtimes/datascience/ubi9-python-3.11/utils/requirements-elyra.txt

harshad16

@dibryant , can you please add fail=0 ; \ below line number 522
as i believe we missed it and line 546 complain 😅

Makefile

runtimes/minimal/ubi9-python-3.11/kustomize/base/requirements-elyra.txt

runtimes/datascience/ubi9-python-3.11/kustomize/base/pod.yaml

./runtimes/rocm-tensorflow/ubi9-python-3.11/kustomize/base/kustomization.yaml Error: 11:1 [empty-lines] too many blank lines (1 > 0) ./runtimes/tensorflow/ubi9-python-3.11/kustomize/base/kustomization.yaml Error: 11:1 [empty-lines] too many blank lines (1 > 0) and wrongly intended line in Makefile, that's not linted automatically

without it, pip would error out on this invalid requirements file

jiridanek · 2024-11-27T08:23:19Z

Checked this with github actions in https://github.com/jiridanek/notebooks/actions/runs/12045657286/job/33585360434, the results look good, only failures there are known issues in cuda/rocm images.

This PR is not removing elyra-requirements.txt from python3.9 images and from the intel runtimes, but that's ok. We want to delete the python3.9 images from main anyways, and we don't touch intel because it's broken (https://issues.redhat.com/browse/RHOAIENG-8388)

/lgtm
/approve
Thank you +💯

openshift-ci · 2024-11-27T08:23:28Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jiridanek

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [jiridanek]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

jiridanek · 2024-11-27T08:25:15Z

/label tide/merge-method-squash

to squash all commits on the PR together

jiridanek · 2024-11-27T10:06:30Z

/override "build (rocm-jupyter-pytorch-ubi9-python-3.11) / build"
/override "build (rocm-jupyter-pytorch-ubi9-python-3.9) / build"

This is due to known issue

Trivy running out of disk space on /tmp #784

openshift-ci · 2024-11-27T10:06:39Z

@jiridanek: Overrode contexts on behalf of jiridanek: build (rocm-jupyter-pytorch-ubi9-python-3.11) / build, build (rocm-jupyter-pytorch-ubi9-python-3.9) / build

In response to this:

/override "build (rocm-jupyter-pytorch-ubi9-python-3.11) / build"
/override "build (rocm-jupyter-pytorch-ubi9-python-3.9) / build"

This is due to known issue

Trivy running out of disk space on /tmp #784

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

jiridanek · 2024-11-27T11:21:33Z

ROCm images are failing to build on ephemeral-storage issues, e.g.

Error on reading termination message from logs: failed to ...-c87b227aef51/docker-build/0.log: no such file or directory, the build jupyter-pytorch-ubi9-python-3.9-amd64 failed after 10m39s with reason BuildPodEvicted: The node was low on resource: ephemeral-storage. Threshold quantity: 32121498671, available: 27985436Ki. Container docker-build was using 1112Ki, request is 0, has larger consumption of ephemeral-storage.

/override ci/prow/rocm-runtimes-ubi9-e2e-tests
/override ci/prow/runtimes-ubi9-e2e-tests

openshift-ci · 2024-11-27T11:21:41Z

@jiridanek: Overrode contexts on behalf of jiridanek: ci/prow/rocm-runtimes-ubi9-e2e-tests, ci/prow/runtimes-ubi9-e2e-tests

In response to this:

ROCm images are failing to build on ephemeral-storage issues, e.g.

Error on reading termination message from logs: failed to ...-c87b227aef51/docker-build/0.log: no such file or directory, the build jupyter-pytorch-ubi9-python-3.9-amd64 failed after 10m39s with reason BuildPodEvicted: The node was low on resource: ephemeral-storage. Threshold quantity: 32121498671, available: 27985436Ki. Container docker-build was using 1112Ki, request is 0, has larger consumption of ephemeral-storage.

/override ci/prow/rocm-runtimes-ubi9-e2e-tests
/override ci/prow/runtimes-ubi9-e2e-tests

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

jiridanek · 2024-11-27T12:16:10Z

/retest

jiridanek · 2024-11-27T15:53:08Z

/test ci/prow/runtimes-ubi9-e2e-tests

…761 (comment) when merging the work

…ebooks Github Actions (#775) * RHOAIENG-16076: tests(gha): run Makefile tests in GitHub Actions * fixup, looks like I lost the second changed line from #761 (comment) when merging the work * fixup, linter wants space in the comments; IntelliJ is ok with it, so let's do that * fixup, add reference to OpenShift CI for the source of the make invocations * fixup, the ifNotPresent pull policy (for PR checks without image registry) and the symbolic links apparently needed to deploy rocm stuff

shalberd · 2024-12-04T14:30:45Z

@atheo89 @jiridanek

Good idea, I've been putting in the required libraries from requirements-elyra.txt upstream into the main requirements file / Pipfile when building Jupyterlab images and runtime images for a long time now.

With that and the commented-out install at runtime in bootstrapper.py upstream notice, the issue is fixed and Elyra has all it needs even at runtime in Airflow and KFP.

I also introduced a new env var that makes output of file run logs optional to S3.
https://github.com/elyra-ai/elyra/blob/main/elyra/kfp/bootstrapper.py#L52
See documentation on that at readthedocs for elyra main / latest:
https://elyra.readthedocs.io/en/latest/user_guide/env-variables-file-based-nodes.html#

…ebooks Github Actions (opendatahub-io#775) * RHOAIENG-16076: tests(gha): run Makefile tests in GitHub Actions * fixup, looks like I lost the second changed line from opendatahub-io#761 (comment) when merging the work * fixup, linter wants space in the comments; IntelliJ is ok with it, so let's do that * fixup, add reference to OpenShift CI for the source of the make invocations * fixup, the ifNotPresent pull policy (for PR checks without image registry) and the symbolic links apparently needed to deploy rocm stuff (cherry picked from commit b3d8af0)

…books Github Actions (opendatahub-io#775) * RHOAIENG-16076: tests(gha): run Makefile tests in GitHub Actions * fixup, looks like I lost the second changed line from opendatahub-io#761 (comment) when merging the work * fixup, linter wants space in the comments; IntelliJ is ok with it, so let's do that * fixup, add reference to OpenShift CI for the source of the make invocations * fixup, the ifNotPresent pull policy (for PR checks without image registry) and the symbolic links apparently needed to deploy rocm stuff (cherry picked from commit b3d8af0)

…ebooks Github Actions (opendatahub-io#775) * RHOAIENG-16076: tests(gha): run Makefile tests in GitHub Actions * fixup, looks like I lost the second changed line from opendatahub-io#761 (comment) when merging the work * fixup, linter wants space in the comments; IntelliJ is ok with it, so let's do that * fixup, add reference to OpenShift CI for the source of the make invocations * fixup, the ifNotPresent pull policy (for PR checks without image registry) and the symbolic links apparently needed to deploy rocm stuff

openshift-ci bot added the do-not-merge/work-in-progress label Nov 4, 2024

openshift-ci bot requested review from atheo89 and paulovmr November 4, 2024 14:18

dibryant changed the title ~~WIP Remove requirments-elyra.txt from the code base~~ [WIP] Remove requirments-elyra.txt from the code base Nov 4, 2024

dibryant force-pushed the rhoaieng-11068 branch from 33dc7a7 to ff9bdeb Compare November 4, 2024 14:29

Remove requirments-elyra.txt from the code base

906368e

dibryant force-pushed the rhoaieng-11068 branch from ff9bdeb to 906368e Compare November 5, 2024 14:16

dibryant changed the title ~~[WIP] Remove requirments-elyra.txt from the code base~~ Remove requirments-elyra.txt from the code base Nov 5, 2024

openshift-ci bot removed the do-not-merge/work-in-progress label Nov 5, 2024

jiridanek reviewed Nov 5, 2024

View reviewed changes

atheo89 reviewed Nov 6, 2024

View reviewed changes

Makefile Outdated Show resolved Hide resolved

jiridanek reviewed Nov 7, 2024

View reviewed changes

Makefile Outdated Show resolved Hide resolved

dibryant force-pushed the rhoaieng-11068 branch 2 times, most recently from 8584bc1 to 6f26350 Compare November 13, 2024 14:21

harshad16 requested changes Nov 15, 2024

View reviewed changes

openshift-ci bot assigned harshad16 Nov 15, 2024

dibryant force-pushed the rhoaieng-11068 branch 2 times, most recently from 5382208 to e8d535f Compare November 19, 2024 13:54

harshad16 requested changes Nov 21, 2024

View reviewed changes

Makefile Outdated Show resolved Hide resolved

dibryant force-pushed the rhoaieng-11068 branch 2 times, most recently from 7e917dd to 2144737 Compare November 22, 2024 23:25

jiridanek reviewed Nov 24, 2024

View reviewed changes

Makefile Show resolved Hide resolved

jiridanek reviewed Nov 24, 2024

View reviewed changes

runtimes/minimal/ubi9-python-3.11/kustomize/base/requirements-elyra.txt Outdated Show resolved Hide resolved

jiridanek added tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. trivy-scan This label that allows trivy to create a security report on the pull requests labels Nov 24, 2024

dibryant force-pushed the rhoaieng-11068 branch 2 times, most recently from 89fda11 to b69c78a Compare November 26, 2024 16:37

jiridanek requested changes Nov 27, 2024

View reviewed changes

runtimes/datascience/ubi9-python-3.11/kustomize/base/pod.yaml Outdated Show resolved Hide resolved

openshift-ci bot assigned jiridanek Nov 27, 2024

changes that were requested in review

a578310

also remove volumeMounts from pod.yaml files

7a0726b

jiridanek force-pushed the rhoaieng-11068 branch from 7f531fa to 8a39449 Compare November 27, 2024 08:03

jiridanek force-pushed the rhoaieng-11068 branch from 8a39449 to b8a628f Compare November 27, 2024 08:07

fix missing comment starter # marks in utils/requirements-elyra.txt

844aaa6

without it, pip would error out on this invalid requirements file

jiridanek force-pushed the rhoaieng-11068 branch from bf3931e to 844aaa6 Compare November 27, 2024 08:14

openshift-ci bot added the lgtm label Nov 27, 2024

openshift-ci bot added the approved label Nov 27, 2024

openshift-merge-bot bot merged commit eff0195 into opendatahub-io:main Nov 27, 2024
51 of 53 checks passed

jiridanek added a commit to jiridanek/notebooks that referenced this pull request Nov 28, 2024

fixup, looks like I lost the second changed line from opendatahub-io#…

d8bc7f3

…761 (comment) when merging the work

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove requirments-elyra.txt from the code base #761

Remove requirments-elyra.txt from the code base #761

dibryant commented Nov 4, 2024 •

edited by jiridanek

Loading

jiridanek Nov 5, 2024

jiridanek Nov 5, 2024

dibryant Nov 5, 2024

atheo89 Nov 6, 2024

harshad16 left a comment

harshad16 Nov 15, 2024

harshad16 left a comment

jiridanek commented Nov 27, 2024

openshift-ci bot commented Nov 27, 2024

jiridanek commented Nov 27, 2024 •

edited

Loading

jiridanek commented Nov 27, 2024

openshift-ci bot commented Nov 27, 2024

jiridanek commented Nov 27, 2024

openshift-ci bot commented Nov 27, 2024

jiridanek commented Nov 27, 2024

jiridanek commented Nov 27, 2024

shalberd commented Dec 4, 2024 •

edited

Loading

Remove requirments-elyra.txt from the code base #761

Remove requirments-elyra.txt from the code base #761

Conversation

dibryant commented Nov 4, 2024 • edited by jiridanek Loading

Description

How Has This Been Tested?

Merge criteria:

jiridanek Nov 5, 2024

Choose a reason for hiding this comment

jiridanek Nov 5, 2024

Choose a reason for hiding this comment

dibryant Nov 5, 2024

Choose a reason for hiding this comment

atheo89 Nov 6, 2024

Choose a reason for hiding this comment

harshad16 left a comment

Choose a reason for hiding this comment

harshad16 Nov 15, 2024

Choose a reason for hiding this comment

harshad16 left a comment

Choose a reason for hiding this comment

jiridanek commented Nov 27, 2024

openshift-ci bot commented Nov 27, 2024

jiridanek commented Nov 27, 2024 • edited Loading

jiridanek commented Nov 27, 2024

openshift-ci bot commented Nov 27, 2024

jiridanek commented Nov 27, 2024

openshift-ci bot commented Nov 27, 2024

jiridanek commented Nov 27, 2024

jiridanek commented Nov 27, 2024

shalberd commented Dec 4, 2024 • edited Loading

dibryant commented Nov 4, 2024 •

edited by jiridanek

Loading

jiridanek commented Nov 27, 2024 •

edited

Loading

shalberd commented Dec 4, 2024 •

edited

Loading