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

Decouple docker images #1371

Merged
merged 17 commits into from
Aug 2, 2022
Merged

Decouple docker images #1371

merged 17 commits into from
Aug 2, 2022

Conversation

iameskild
Copy link
Member

@iameskild iameskild commented Jul 15, 2022

Fixes | Closes | Resolves - related to #1292

Please remove anything marked as optional that you don't need to fill in.
Choose one of the keywords preceding to refer to the issue this PR solves, followed by the issue number (e.g Fixes # 666).
If there is no issue, remove the line. Remove this note after reading.

Changes introduced in this PR:

  • Now that the docker images have been transferred to its own repo, we can remove the references here. As a result, a lot of code is being removed.
  • With the images transferred, there are few minor changes that were needed in order to properly set the tags for the jupyterlab, jupyterhub and dask-worker images.
    • for testing (kubernetes.yaml), we will pull the latest image tagged from the main branch of nebari-docker-images to ensure that everything from both repos works together. I believe this preserves the spirit of how the tags were set prior to this change.
    • for qhub init, a small function (set_qhub_image_tag) will download a list of the GitHub tags for the nebari-docker-images repo and use the latest (no dev or pre-releases) tag.
      • A similar function has been created to set the qhub_dask_version as well.
    • in both cases, these image tags can be overridden by setting the QHUB_IMAGE_TAG environment variable.
      • A similar env var (QHUB_DASK_VERSION) has been introduced to override the qhub_dask_version.

Types of changes

What types of changes does your PR introduce?

Put an x in the boxes that apply

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds a feature)
  • Breaking change (fix or feature that would cause existing features to not work as expected)
  • Documentation Update
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no API changes)
  • Build related changes
  • Other (please describe):

Testing

Requires testing

  • Yes
  • No

In case you checked yes, did you write tests?

  • Yes
  • No

Documentation

Does your contribution include breaking changes or deprecations?
If so have you updated the documentation?

  • Yes, docstrings
  • Yes, main documentation
  • Yes, deprecation notices

Further comments (optional)

If this is a relatively large or complex change, kick off the discussion by explaining why you chose the solution you did and what alternatives you considered and more.

@iameskild iameskild changed the title [WIP] Use nebari docker images Decouple docker images Jul 17, 2022
.github/workflows/kubernetes_test.yaml Show resolved Hide resolved
qhub/cli/__init__.py Outdated Show resolved Hide resolved
qhub/provider/cicd/common.py Outdated Show resolved Hide resolved
qhub/provider/cicd/github.py Outdated Show resolved Hide resolved
qhub/utils.py Outdated Show resolved Hide resolved
qhub/utils.py Outdated Show resolved Hide resolved
@trallard
Copy link
Member

Added a few comments, nothing super major @iameskild, thanks for sorting this out

@trallard trallard added needs: review 👀 This PR is complete and ready for reviewing needs: changes 🧱 Review completed - some changes are needed before merging and removed needs: review 👀 This PR is complete and ready for reviewing labels Jul 19, 2022
@iameskild iameskild requested a review from trallard July 29, 2022 00:57
@iameskild iameskild removed the needs: changes 🧱 Review completed - some changes are needed before merging label Jul 29, 2022
@iameskild
Copy link
Member Author

@trallard I believe this ready for a final review. Once this PR is merged, all future updates to the images will need to be made in the nebari-docker-images repo 👍

Copy link
Member

@costrouc costrouc left a comment

Choose a reason for hiding this comment

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

Looks like a good solution moving forward for updating the nebari image tags. Happy to see that we are decoupling the docker images from Nebari!

@iameskild iameskild merged commit e6d64d9 into main Aug 2, 2022
@iameskild iameskild deleted the use-nebari-docker-images branch August 2, 2022 19:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Development

Successfully merging this pull request may close these issues.

3 participants