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 the marble image to notebook images list #427

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dchandan
Copy link
Collaborator

Overview

Adding the newly created marble Jupyter image to the notebook image selections list.

Changes

Non-breaking changes

  • Changes are made to env.local.example

birdhouse_daccs_configs_branch: master
birdhouse_skip_ci: false

@crim-jenkins-bot
Copy link
Collaborator

Can one of the admins verify this patch?

@dchandan dchandan changed the title adding the marble image to notebook images list Adding the marble image to notebook images list Feb 27, 2024
@@ -263,7 +263,8 @@ export GEOSERVER_ADMIN_PASSWORD="${__DEFAULT__GEOSERVER_ADMIN_PASSWORD}"
#export ALLOW_UNSECURE_HTTP=""

# Jupyter single-user server images
#export DOCKER_NOTEBOOK_IMAGES="pavics/workflow-tests:210216 \
#export DOCKER_NOTEBOOK_IMAGES="marbleclimate/marble-jupyter-images \
Copy link
Collaborator

Choose a reason for hiding this comment

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

Where is this hosted? I cannot find them on DockerHub (default for docker pull). Is it on a private/other registry?
If a user enables this as is without appropriate docker login to the appropriate registry, that will make the deployment fail due to unresolved reference, unless the Jupyter config is overridden here to allow missing dockers :

c.DockerSpawner.pull_policy = "always" # for images not using pinned version

Also, just to make sure, is the trailing s of the image a typo? Is it a single docker image using this name explicitly? Images should be listed individually here.

Copy link
Collaborator

@mishaschwartz mishaschwartz left a comment

Choose a reason for hiding this comment

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

Actually this is a discussion that I've been wanting to have for a while and this is a good place to start it!

I think that we should move this into a marble specific component (under optional-components) that includes all of the marble specific updates and overrides that we want to recommend/enforce for Marble nodes. That way we have a space to update those sort of changes without over-customizing this repo for Marble (like it previously was for PAVICS).

What do you think?

@tlvu
Copy link
Collaborator

tlvu commented Feb 27, 2024

Actually this is a discussion that I've been wanting to have for a while and this is a good place to start it!

I think that we should move this into a marble specific component (under optional-components) that includes all of the marble specific updates and overrides that we want to recommend/enforce for Marble nodes. That way we have a space to update those sort of changes without over-customizing this repo for Marble (like it previously was for PAVICS).

What do you think?

Right. If the goal is not just providing an example but to enforce it then yes optional-component is the way to go. Having an optional-component will also allow you to control future updates. When updating a sample code in env.local.example, that change will not propagate to users have enabled the previous sample code.

Or another repo entirely to keep it clean and tight. Ouranos very specific overrides/additions are here https://github.com/bird-house/birdhouse-deploy-ouranos.

@fmigneault
Copy link
Collaborator

If the goal is only to apply overrides, then a separate repo like the one mentioned by @tlvu seems more appropriate. CRIM has similar private repos for env.local and a few extra configs.

However, it is important to use such repo only for overrides. If new capabilities are added, they should be brought back into birdhouse-deploy as components/optional-components as appropriate.

@mishaschwartz
Copy link
Collaborator

However, it is important to use such repo only for overrides. If new capabilities are added, they should be brought back into birdhouse-deploy as components/optional-components as appropriate.

Of course, we promise not to hoard new features

@dchandan
Copy link
Collaborator Author

It seems to me that there is a general consensus to keep birdhouse-deploy generic and not make it too tied to the Marble platform. I also think this is a good idea. With regards to whether we want to have Marble specific overrides as an optional component or a specific repo, I think both are good ways to go, though I am more inclined to favour the separate repo option as that would provide an easier approach to offer a more consistent branding and messaging related to Marble and provide a dedicated place for documenting the relationship between the generic birdhouse-deploy and the version of it that is configured for Marble.

Though, I think in this approach we'd need two override repos (i) the Marble specific override repo and (ii) another override repo containing local configuration for each organization deploying a node. But I think this is OK.

Lastly, I'd like to point out that regardless of which approach we take to incorporate Marble into birdhouse in a generic way, there are still several hardcoded references to PAVICS and PAVICS configuration, not to mention other things like pavics-compose.sh. If we are trying to separate our work (PAVICS-Marble) from birdhouse-deploy, then it would only be a half-measure effort to put the Marble specific information into a separate repo or in optional components, without also removing all the PAVICS references. It addition to this being an incomplete effort, it would also be incoherent and lead to confusion for others down the line: PAVICS is part of Marble, so why is Marble separated in a particular way, but the fingerprints of PAVICS are left all over birdhouse-deploy? There is no good answer to this.

So, I think that before separating Marble config into an optional-component or separate repo, we really need to get rid of birdhouse-deploy with references to PAVICS. Yes, this needs effort, but it is in my opinion long overdue.

@mishaschwartz
Copy link
Collaborator

there are still several hardcoded references to PAVICS and PAVICS configuration

For sure! That is being worked on in #428

@mishaschwartz mishaschwartz self-assigned this Mar 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants