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

Update VS Code Server Notebook #452

Merged
merged 1 commit into from
Mar 14, 2024
Merged

Conversation

atheo89
Copy link
Member

@atheo89 atheo89 commented Mar 12, 2024

Related to: https://issues.redhat.com/browse/RHOAIENG-4345

Moreover seems that solves also this: https://issues.redhat.com/browse/RHOAIENG-2312

NOTE: When I launch a new notebook, the interpreter is already set to /opt/app-root/bin/python3, located in the bottom right corner.
image

Description

This pull request updates the following components:

  • The codeserver version from v4.16.1 to v4.22.0

  • The pre-built extensions

    • ms-python.python from v2023.14.0 to v2024.2.1
    • ms-toolsai.jupyter from v2023.3.100 to v2023.9.100
  • The pre-built python packages

    • Pipfile link
      NOTE: The kafka-python library version ~=2.0.2 that we preinstall has ceased updates since September 30, 2020. Considering this, should we include it, or would it better to avoid it due to the potential introduction of vulnerabilities in the notebook?
  • Update the codeserver related ImageStream
    We drop the n-2 on upstream

How Has This Been Tested?

Way 1

  • Run podman run --network=host --name validation-container quay.io/opendatahub/workbench-images:codeserver-ubi9-python-3.9-pr-452
  • Inspect the notebook
  • Run a Python code snippet without manually setting the interpreter. (This is related to evaluation RHOAIENG-4345).
import numpy as np

# Create a 1D NumPy array
arr = np.array([1, 2, 3, 4, 5])

# Print the array
print("Original array:", arr)

# Perform some basic operations
print("Sum of elements:", np.sum(arr))
print("Mean of elements:", np.mean(arr))
print("Maximum element:", np.max(arr))
print("Index of maximum element:", np.argmax(arr))

Way 2

  • Import the notebook in ODH
    image
  • Lunch the newly added notebook and run the above piece of code, you should see the following
    image

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

@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.

looks pretty neat
amazing work.
lets remove the entire code-server for centos, we would only rely on ubi9 one.

Copy link
Member

@jiridanek jiridanek left a comment

Choose a reason for hiding this comment

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

I played with the UBI9 version of the image and it's working and it resolves several issues in issues.redhat.com that we have there. +1 from me!

Copy link
Contributor

openshift-ci bot commented Mar 13, 2024

@jiridanek: changing LGTM is restricted to collaborators

In response to this:

I played with the UBI9 version of the image and it's working and it resolves several issues in issues.redhat.com that we have there. +1 from me!

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/test-infra repository.

Copy link
Contributor

@rkpattnaik780 rkpattnaik780 left a comment

Choose a reason for hiding this comment

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

Verified with the instructions provided.

Copy link
Contributor

openshift-ci bot commented Mar 14, 2024

@atheo89: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/images c3d1431 link true /test images
ci/prow/notebooks-e2e-tests c3d1431 link true /test notebooks-e2e-tests
ci/prow/runtime-intel-pyt-ubi9-python-3-9-pr-image-mirror c3d1431 link true /test runtime-intel-pyt-ubi9-python-3-9-pr-image-mirror
ci/prow/notebook-jupyter-intel-pyt-ubi9-python-3-9-pr-image-mirror c3d1431 link true /test notebook-jupyter-intel-pyt-ubi9-python-3-9-pr-image-mirror
ci/prow/runtime-intel-tf-ubi9-python-3-9-pr-image-mirror c3d1431 link true /test runtime-intel-tf-ubi9-python-3-9-pr-image-mirror
ci/prow/notebook-jupyter-intel-tf-ubi9-python-3-9-pr-image-mirror c3d1431 link true /test notebook-jupyter-intel-tf-ubi9-python-3-9-pr-image-mirror

Full PR test history. Your PR dashboard.

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/test-infra repository. I understand the commands that are listed here.

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.

Excellent work

/lgtm
/approve

thanks 💯

@openshift-ci openshift-ci bot added the lgtm label Mar 14, 2024
Copy link
Contributor

openshift-ci bot commented Mar 14, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: harshad16, jiridanek, rkpattnaik780

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

@harshad16 harshad16 merged commit c0a4639 into opendatahub-io:main Mar 14, 2024
2 of 10 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.

None yet

4 participants