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.

Sign up for GitHub

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

Add a single init_container instead of overwriting init_containers #18

Merged
merged 1 commit into from
Jun 2, 2021
Merged

Add a single init_container instead of overwriting init_containers #18

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions persistent_binderhub/files/jupyterhub/persistent_bhub_config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,13 +195,13 @@ def start(self):
# because start command is run as an ENTRYPOINT and initcontainer's command overwrites it
# But start command will be executed in notebook container (because we dont define a custom command for it),
# so change will take place there, and on user's side, there is no problem
self.init_containers = [{
self.init_containers.append({
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, just not sure, but will start() be called only once per instance?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe so, a subclass with start

def start(self):
     self.init_containers.append(another_container_spec)
     super().start()

should work IMO.

Copy link
Contributor

Choose a reason for hiding this comment

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

I just am not sure, if it is possible that the same instance calls start() more than once. In this case, we would end up with 2 identical init containers. But if this is not possible, everything is fine

Copy link
Contributor Author

Choose a reason for hiding this comment

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

IMO this should be okay (we can always revert this if something breaks :) )

This is how the subclassing would look like

In [1]: class A:  # KubeSpawner
   ...:     def __init__(self):
   ...:         self.a = list()
   ...:     def start(self):
   ...:         self.a.append('something')
   ...:

In [2]: class B(A):  # PersistentBinderSpawner
   ...:     def start(self):
   ...:         self.a.append('foo')
   ...:         super().start()
   ...:

In [3]: class C(B):
   ...:     def start(self):
   ...:         self.a.append('bar')
   ...:         super().start()
   ...:

In [4]: test = C()

In [5]: test.a
Out[5]: []

In [6]: test.start()

In [7]: test.a
Out[7]: ['bar', 'foo', 'something']

Copy link
Contributor

@g-braeunlich g-braeunlich Jun 2, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Still to make my concerns clear: What I fear is the following:

test = C()
test.start()
...
test.start()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah yes, that would create copies.
But it's getting called once, assuming the custom spawner is set using

c.JupyterHub.spawner_class = CustomSpawner

"name": "project-manager",
"image": self.image,
"command": command,
# volumes is already defined for notebook container (self.volumes)
"volume_mounts": [projects_volume_mount],
}]
})

# notebook container (user server)
# mount all projects (complete user disk) to /projects
Expand Down