jupyterhub / wrapspawner

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

Reconsidering manual server linkage #54

Closed rcthomas closed 6 months ago

rcthomas commented 2 years ago

In #51 we added a property linking the child spawner's server. About the same time, jupyterhub/jupyterhub#3810 was merged to keep Spawner.server in sync with underlying orm_spawner.server and that PR was referenced in the discussion on #50 and #51. Folks tested #50 and #51 against JupyterHub setups that I don't think included the server sync fix.

While catching up on my deployment this week I noticed I was still running off the branch in #50 (with the strange hack), and when I tried to run off current wrapspawner master it failed to spawn properly. Specifically, during spawn the Hub reports that it expects the spawner on the wrong URL (as reported from here), namely a URL that corresponds to the hub itself.

In the discussion on #50 Min mentioned that the server sync fix may be all that was necessary so I decided to follow-up on that conjecture, and I reverted #51 (equivalent to just running the latest release, 1.0.1), and for me things now seem to be working again. I think #51 somehow conflicts with jupyterhub/jupyterhub#3810 but may be obviated by it anyway.

We probably want to consider reverting #51, but I think others should test. Basically the question is whether #51 breaks anything for anyone running JupyterHub 2.2.0 or later.

rcthomas commented 2 years ago

I've tested JupyterHub versions 2.0.2, 2.1.1, 2.3.1 are all OK with wrapspawner 1.0.1. But those versions are not OK with the most recent commit that adds the server properties. These tests were fairly easy because I just walked back my existing JupyterHub 2.x configs, but I'm wondering if someone else could check on say the latest Hub 1.x the same way.

alibi1125 commented 2 years ago

I can confirm that #51 breaks (and reverting it fixes) the spawning behavior on JupyterHub 2.3.1 for me as well. Can't say anything for JupyterHub 1.x, though. If you want me to do some further digging or require additional info on our setup, please let me know.

octomike commented 1 year ago

This day's rabbit hole brought me here - upgraded to JupyterHub 3.0.0 today and found this is still an issue.

rcthomas commented 1 year ago

@octomike could you give us a little more info there?

octomike commented 1 year ago

Sure thing!

I moved from an ancient JH 1.1.0 and I tried wrapspawner@master first.

rcthomas commented 1 year ago

OK, and you'd been using that version of wrapspawner before the upgrade? Thanks!

octomike commented 1 year ago

OK, and you'd been using that version of wrapspawner before the upgrade? Thanks!

Nope, we also had an older one (versioned as 0.0.1.dev0) before.

Ph0tonic commented 9 months ago

Hi, I confirm that reverting #51 is necessary and that wrapspawner works correctly without this patch. Here is a PR to reverse this:

It would be nice to be able to move forward. :+1:

\cc @mbmilligan