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 the runtime image with the commit: 8bda2fa on main #65

Merged
merged 1 commit into from
Oct 27, 2023

Conversation

atheo89
Copy link
Member

@atheo89 atheo89 commented Oct 23, 2023

Update the runtime image with the commit: 8bda2fa on main

@openshift-ci
Copy link

openshift-ci bot commented Oct 24, 2023

@atheo89: The following test 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 5e7c9fd link true /test images

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

@jstourac jstourac left a comment

Choose a reason for hiding this comment

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

modh is probably another new organisation in quay.io for me. Do we have it documented somewhere which quay orgs serves to what? 🙃

Also, links to opendatahub/workbench-images:runtime* in these Dockerfiles are okay? Namely the values of io.openshift.build.image, io.openshift.build.source-location and authoritative-source-url

Regarding the Tensorflow - I don't like that the naming here doesn't match convention with the others - the runtime keyword isn't first. Is it possible to change this paradigm?

Doesn't need to be addressed in this PR, we can create an issue for that to track it if we agree.

@@ -3,7 +3,7 @@
"metadata": {
"tags": [],
"display_name": "Python 3.8 (UBI8)",
"image_name": "quay.io/opendatahub/workbench-images:runtime-minimal-ubi8-python-3.8-6a6098d"
"image_name": "quay.io/modh/runtime-images@sha256:a3ee8b8eff99e9699fba1c1a51a9eedc4499caceeb4106e708da048ea0c30ef3"
Copy link
Member

Choose a reason for hiding this comment

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

This Anaconda image points to the minimal one - is this on purpose?

Copy link
Member

Choose a reason for hiding this comment

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

yes this is intentional
these are runtime image, directly doesn't have conda in them
they would be used by elyra pipeline, if someone executes them.

@harshad16
Copy link
Member

modh is probably another new organisation in quay.io for me. Do we have it documented somewhere which quay orgs serves to what? 🙃

Also, links to opendatahub/workbench-images:runtime* in these Dockerfiles are okay? Namely the values of io.openshift.build.image, io.openshift.build.source-location and authoritative-source-url

Regarding the Tensorflow - I don't like that the naming here doesn't match convention with the others - the runtime keyword isn't first. Is it possible to change this paradigm?

Doesn't need to be addressed in this PR, we can create an issue for that to track it if we agree.

Documenting about the images, yes we need to do it
lets improve on that.

about the Dockerfiles labels. we should improve those as well.
will include it as a task.

Naming convention, i wouldn't change it anymore, as it is only on dev side, and users would see display name.

@jstourac
Copy link
Member

jstourac commented Oct 25, 2023

Documenting about the images, yes we need to do it lets improve on that.

opendatahub-io#285

about the Dockerfiles labels. we should improve those as well. will include it as a task.

well, I was about to create an issue for that but I'm not sure whether we shall create it in upstream 🤔
update: created as opendatahub-io#292

Naming convention, i wouldn't change it anymore, as it is only on dev side, and users would see display name.

okay 🙃

@harshad16
Copy link
Member

/lgtm
/approve
thanks 👍

@openshift-ci openshift-ci bot added the lgtm label Oct 27, 2023
@harshad16 harshad16 merged commit 15f8a5c into red-hat-data-services:main Oct 27, 2023
3 of 5 checks passed
@openshift-ci
Copy link

openshift-ci bot commented Oct 27, 2023

[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

@atheo89 atheo89 deleted the fixes-on-runtimes branch October 23, 2024 08:01
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.

3 participants