jupyterhub / wrapspawner

Mechanism for runtime configuration of spawners for JupyterHub
BSD 3-Clause "New" or "Revised" License
60 stars 58 forks source link

Restore child_config settings overwritten by directional_link #39

Closed cexen closed 4 years ago

cexen commented 4 years ago

Fixes #30.

rcthomas commented 4 years ago

Calling @cmd-ntrf:

@mbmilligan and I were talking about this PR and the associated issue this week on Zoom. The way the directional link works is it overrides the child spawner settings. But if you have a list of profiles, you have to leave the parent setting alone and define it for all of them in the child spawner specification. I think this PR lets the parent setting propagate (so if it's not set for a child it gets defined by the parent) but if it is set, it brings back the child setting.

It works that way but I wondered if you looked at this short PR you might weigh in on whether it looks OK like it breaks anything you intended?

cmd-ntrf commented 4 years ago

I think it's ok and does not break what I had in mind with directional links.

However, I would opt for a bit simpler approach: removing child_config keys from the set of keys that are common between the parent and the child when building the set of traits.

So instead of adding an if in the for loop, subtract the keys of self.child_config from the intersection of the parent and the child traits like this:

common_traits = (
  set(self._trait_values.keys()) &
  set(self.child_spawner._trait_values.keys()) - 
  set(self.child_config.keys())
)
rkdarst commented 4 years ago

I found I had to use this to make the default_url attribute be passed to the spawner. I haven't looked into much detail about why this is, but without it my general update didn't work at all because of the new way of starting jupyterlab (which requires setting default_url).