Default base image config#802
Conversation
Signed-off-by: Adam Maly <amaly@redhat.com>
Signed-off-by: Adam Maly <amaly@redhat.com>
Signed-off-by: Adam Maly <amaly@redhat.com>
Signed-off-by: Adam Maly <amaly@redhat.com>
Signed-off-by: Adam Maly <amaly@redhat.com>
jesuino
left a comment
There was a problem hiding this comment.
Hello Adam,
Thanks for your work on this. Here are a few suggested small changes. Please let me know what you think.
| if (metadata.base_image === '') { | ||
| metadata.base_image = DefaultState.metadata.base_image; | ||
| } | ||
| metadata.base_image = resolvedDefaultBaseImage; |
There was a problem hiding this comment.
What if user had already a base_image in place, wouldn't this be overriding the user setting?
There was a problem hiding this comment.
I did this on purpose, I think we should not consider the default base image from metadata.
It would be confusing for user if he sets some default image in the settings or by env var, but because he is using maybe an old notebook with base_image in metadata the settings or the env var never apply.
Signed-off-by: Adam Maly <amaly@redhat.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
@jesuino thanks for the review. I fixed most of the things and left one comment that should answer your review question. |
jesuino
left a comment
There was a problem hiding this comment.
Hello @ada333 Thanks for the changes, would you please fix the merge conflicts?
One last thing is that I think that the DEFAULT_BASE_IMAGE from constants.ts is not being used anymore, would you please check it and if it is not used remove it?
Thanks!
Signed-off-by: Adam Maly <amaly@redhat.com>
Signed-off-by: Adam David Maly <42520598+ada333@users.noreply.github.com>
Signed-off-by: Adam Maly <amaly@redhat.com>
Signed-off-by: Adam Maly <amaly@redhat.com>
|
@jesuino thanks for review, I resolved the conflicts and deleted the constant. |
This PR solves #727
Adds env var (DEFAULT_BASE_IMAGE) and settings (with settings having priority) support of configuring default base image for all steps.
So now the resolution of image for each step should have this order:
per-step specified image > jupyterlab setting > env var > hardcoded default python 3.12
Also now in image dialog the default image should be visible:

Also I noticed some unused legacy code mainly in podutils.py and Commands.ts and took a chance to delete it since the functions were never called and I think we will not have use for them.
also I had to adjust gitignore because it was ignoring the lib/ folder in which we have lot of utils