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

Adding jupyterlab-conda-store extension support to Nebari #1564

Merged
merged 1 commit into from
Jan 5, 2023

Conversation

costrouc
Copy link
Member

This PR depends on
nebari-dev/nebari-docker-images#41 being merged.

Additionally fixes an unupdated section where we specified an older conda-store version.

We override the conda-store url in the jupyterlab settings overrides.

Reference Issues or PRs

What does this implement/fix?

  • 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 not to work as expected)
  • Documentation Update
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no API changes)
  • Build related changes
  • Other (please describe):

This PR depends on
nebari-dev/nebari-docker-images#41 being merged.

Additionally fixes an unupdated section where we specified an older
conda-store version.

We override the conda-store url in the jupyterlab settings overrides.
@costrouc
Copy link
Member Author

This PR requires using the jupyterlab image from nebari-dev/nebari-docker-images#41. I have not tested this yet. Will once other is merged.

Copy link
Member

@iameskild iameskild left a comment

Choose a reason for hiding this comment

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

LGTM :)

@@ -36,7 +36,7 @@ variable "conda-store-image" {
variable "conda-store-image-tag" {
description = "Version of conda-store to use"
type = string
default = "v0.4.9"
default = "v0.4.12"
Copy link
Member

Choose a reason for hiding this comment

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

A few weeks ago, I added the default conda-store image tag as an input here. This was part of the motivation for this RFD :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeup I saw that it had been updated in constants. Not sure if we should even have a default value here then?

Copy link
Member

Choose a reason for hiding this comment

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

Removing it works for me 👍

@pavithraes
Copy link
Member

@costrouc @iameskild Shall we merge this PR?

@pavithraes pavithraes added the status: approved 💪🏾 This PR has been reviewed and approved for merge label Dec 13, 2022
@costrouc
Copy link
Member Author

@pavithraes this PR can't be merged until nebari-dev/nebari-docker-images#45 is merged in docker-images. @iameskild would you be fine with the jupyterlab extension fitting into the December release?

@pavithraes pavithraes added the status: blocked ⛔️ This item is on hold due to another task label Dec 13, 2022
@iameskild
Copy link
Member

@costrouc the docker-image PR has been approved and merged :)

@iameskild iameskild removed the status: blocked ⛔️ This item is on hold due to another task label Dec 13, 2022
@iameskild iameskild changed the base branch from release/2022.11.1 to develop December 13, 2022 23:42
@pavithraes
Copy link
Member

@costrouc @iameskild We decided to not have a release this week, but should we still merge this PR?

@costrouc costrouc merged commit 9dbe480 into develop Jan 5, 2023
@costrouc costrouc deleted the jupyterlab-conda-store branch January 5, 2023 16:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: integration/conda-store status: approved 💪🏾 This PR has been reviewed and approved for merge type: enhancement 💅🏼 New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants