Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RHOAIENG-12535: Setup Jupyter 4 + Elyra environment on RHOAI/ODH #701

Merged
merged 8 commits into from
Sep 24, 2024

Conversation

paulovmr
Copy link
Contributor

Description

  • Prepare the notebooks images to use the next odh-elyra release.

  • Remove commented code for Elyra patches that are being applied directly on ODH Elyra fork here.

  • To do:

    • Confirm next version for odh-elyra release on PyPI
    • Update lock files

How Has This Been Tested?

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

Copy link
Member

@atheo89 atheo89 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

Just two comments to revert some comments back 🙂

jupyter/datascience/ubi9-python-3.11/Dockerfile Outdated Show resolved Hide resolved
@jiridanek
Copy link
Member

The CI fails are legit, please fix CI before removing the wip label.

(Just noticed we have two different wip labels in this project defined ;)

@paulovmr
Copy link
Contributor Author

/retest

@atheo89
Copy link
Member

atheo89 commented Sep 18, 2024

Hey Paulo, could you rebase this PR and update also pipfile for TrustyAI notebook?

Copy link
Member

@harshad16 harshad16 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@paulovmr
Copy link
Contributor Author

/retest

1 similar comment
@paulovmr
Copy link
Contributor Author

/retest

Copy link
Member

@harshad16 harshad16 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm
/approve

Reason to approve:

  • The changes bring in elyra back, so it can tested in proper manner
  • The PR builds get override, if new changes are pushed, so lets get this one in.
  • another note: as presubmit and postsubmit jobs get different cluster, some build would be faster in the main than in PR

@openshift-ci openshift-ci bot added the lgtm label Sep 20, 2024
Copy link
Contributor

openshift-ci bot commented Sep 20, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: harshad16

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:

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

@jstourac
Copy link
Member

/lgtm /approve

Reason to approve:

  • The changes bring in elyra back, so it can tested in proper manner
  • The PR builds get override, if new changes are pushed, so lets get this one in.
  • another note: as presubmit and postsubmit jobs get different cluster, some build would be faster in the main than in PR

I'm all for this. Just wondering whether we shouldn't use the latest dev1 Elyra release?

@paulovmr
Copy link
Contributor Author

@jstourac Good point, since I'm rebasing it to fix conflicts, I'm also updating odh-elyra to use 4.0.0.dev1 version.

@paulovmr
Copy link
Contributor Author

/retest

1 similar comment
@paulovmr
Copy link
Contributor Author

/retest

@atheo89
Copy link
Member

atheo89 commented Sep 24, 2024

/override ci/prow/images
Since it fails on ubi8 that we recently removed, the rest looks good
/override ci/prow/notebooks-ubi9-e2e-tests
/override ci/prow/rocm-notebooks-e2e-tests
/lgtm

@openshift-ci openshift-ci bot added the lgtm label Sep 24, 2024
Copy link
Contributor

openshift-ci bot commented Sep 24, 2024

@atheo89: Overrode contexts on behalf of atheo89: ci/prow/images, ci/prow/notebooks-ubi9-e2e-tests, ci/prow/rocm-notebooks-e2e-tests

In response to this:

/override ci/prow/images
Since it fails on ubi8 that we recently removed, the rest looks good
/override ci/prow/notebooks-ubi9-e2e-tests
/override ci/prow/rocm-notebooks-e2e-tests
/lgtm

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.

@openshift-merge-bot openshift-merge-bot bot merged commit aa5049d into opendatahub-io:main Sep 24, 2024
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants