Attempted fix for minor JupyterHub compatibility issue.

[krinsman]

This is an attempt to address the TODO in /packages/hub-extension/src/index.ts.

[jupyterlab-dev-mode]

Thanks for making a pull request to JupyterLab!

To try out this branch on binder, follow this link:

[rcthomas]

We actually rely on named servers in our JupyterHub deployment, so this is actually presenting an issue right now.

[jasongrout]

Do we know how widespread jupyterhub 1.0 is? This seems like it demands jhub 1.0, so we might not be able to do it until jupyterlab 2.0 for backwards compatibility. Is there a way to support jupyterhub 1.0 and previous jupyterhub?

[blink1073]

I think we need a guard here for hasattr(nbapp, 'server_name')

[krinsman]

My understanding was that the default server name is the empty string (as @rcthomas said). If URLExt.join is smart and ignores empty strings as a no-op then presumably such a switch would be unnecessary. That being said I am not sure.

[krinsman]

See here: https://github.com/jupyterhub/jupyterhub/blob/master/jupyterhub/singleuser.py#L449

[krinsman]

If the issue is that the server wouldn't have that attribute if it was just a single person running JupyterLab on their laptop, then why aren't guards hasattr(nbapp, 'hub_prefix'), hasattr(nbapp, 'hub_host'), and hasattr(nbapp, 'user') also necessary? I'm a little confused.

If the issue is that it might not have that attribute when JupyterHub is being run, I don't think that's possible although I guess better safe than sorry.

[blink1073]

What I meant is to check whether the property exists as a way to detect whether we are using jupyterhub 1.0 or not.

[krinsman]

Alright then it seems like it might not be necessary since the server_name attribute existed pre-1.0. jupyterhub/jupyterhub#2089

I think it was called name before my PR (although I'm not 100%), but it was in master at least as early as September 2018.

[jasongrout]

it was in master at least as early as September 2018.

FYI, it looks like that PR was merged and released starting with 1.0 (jupyterhub/jupyterhub@f2fa067 - see the tags that contain that commit)

[krinsman]

@jasongrout Oh oops, you're right. I'm not very good with understanding how the versioning for these things work. Just because it was merged doesn't mean it was in the current most recent release for that time. It does make sense, since the UI for named servers only showed up with 1.0 and not any earlier versions.

Now I feel dumb. I thought changing the name from name to server_name would make the code easier to understand/work with, but that seems to not have been the case. Anyway @blink1073 is definitely right about needing the hasattr. Sorry about that; I think I'm getting overly defensive/insecure about the changes I made/proposed.

[blink1073]

What if we switch here on whether there is a server name and choose the simplified url if there isn't one? Does the simplified url work in jupyterhub 1.0?

[krinsman]

Nope sorry I think I was wrong: https://github.com/jupyterhub/jupyterhub/blob/1a26c1fb817f44acffd3907d0ec94d7de115c2e6/jupyterhub/handlers/pages.py#L131
You're right that switch is the right approach as far as I can tell.

[rcthomas]

@jasongrout The fix also needs to work for JupyterHub>=1 where the JupyterHub config doesn't have named servers turned on (it's off by default). One way of looking at it is that the default server already has a name, it's just "" (empty string).

We're going to try to get a test set up of this hopefully today and I'm thinking we'll test against JupyterHub 1 with named servers turned off to make sure everything is right. That said, what version of Hub would be late enough for compatibility? 0.9.6? Thanks!

[jasongrout]

My big concern is that people may have deployed jlab 1.0 already to jhub 0.x, and it stops working in a minor update to jlab. What version of jhub do we support currently? 0.9.0 was released a year ago - is there any difference in supporting 0.9.0 vs 0.9.6? A year seems like a reasonable support timeline.

[jasongrout]

(We should make our support for a specific version of jhub explicit in the docs somewhere, and perhaps even in the setup.py and/or conda recipe if we can)

[blink1073]

what I had suggested would support both pre and post 1.0 of jupyterhub, but I agree in principle about being explicit.

[krinsman]

I am not sure if JupyterHub compatibility is actually an issue, since JupyterHub has supported named servers via an API (and thus the URLs) for a while, with the default server name just being the empty string (e.g. when named servers are turned off or none exist). The only change in 1.0 is the existence of a UI for named servers. jupyterhub/jupyterhub#2089

That having been said the important thing for me to do right now first would be to test this to make sure it works the way I expect/think it does.

EDIT: See this line in the JupyterHub source -- the server_name attribute always exists, and is the empty string by default: https://github.com/jupyterhub/jupyterhub/blob/master/jupyterhub/singleuser.py#L449

[krinsman]

How does URLExt.join work? If say token2 = '', i.e. the second/"middle" string is the empty string, does it return token1//token3 or token1/token3?

I.e. what is URLExt.join('token1', '', 'token3')?

My guess is that there'd be a problem with the PR in the former case but not necessarily the latter.

[tslaton]

@krinsman was reporting some issues on NERSC's deployment running this PR that I wasn't seeing on my separate fork. I think the only differences between this PR and my fork were that I'd added the hasattr guard and restart url switch @blink1073 suggested on my fork.

Now that I have gained push rights here, I went ahead and implemented those changes so that at least @krinsman and I are on the same page. I still think it's worth considering his comments above questioning what makes these changes necessary.

EDIT: To be clearer, he was reporting trouble launching lab to begin with, whereas I have only seen trouble with server restarts (which these changes have yet to remedy)

[blink1073]

If y'all get it working with both JupyterHub 1.0 and pre-1.0, I'll make a backport release of JupyterLab 1.0 with this change. Thanks for working on this!

[tslaton]

I think there were some bugs @rcthomas resolved in jupyterhub/jupyterhub#2682 and issues with our setup at NERSC that were making it difficult to test the behavior of the restart flow due to the JupyerLab code in isolation before.

Now that I am able to play with it, I don't think the following code is quite right (as proposed):

  const restartUrl = hubServerName
	    ? hubHost + URLExt.join(
	        hubPrefix,
	        'spawn',
	        hubUser,
	        hubServerName,
	        `?next=${hubPrefix}home`
	      )
	    : hubHost + URLExt.join(hubPrefix, `spawn?next=${hubPrefix}home`);

When hubServerName is '', the fallback url is working as intended. But when there is a hubServerName, I'm getting a 404 error when navigating to restartUrl.

Removing ?next=${hubPrefix}home seems to fix the 404 error and give the behavior I was expecting to see (the named server that has stopped restarts in a new tab). So, I think the correct code should be:

  const restartUrl = hubServerName
	    ? hubHost + URLExt.join(
	        hubPrefix,
	        'spawn',
	        hubUser,
	        hubServerName
	      )
	    : hubHost + URLExt.join(hubPrefix, `spawn?next=${hubPrefix}home`);

I don't yet have a good understanding of why we might want to keep the next portion in the url when using named servers, so I'm willing to accept viewpoints as to why this isn't correct here. Otherwise, I can make this small change to the PR, and I think it'd be ready to go.

[blink1073]

What is the behavior with a named server when you do not specify next? It seems like you'd want to go to the home of that specific named server. If that is the behavior with the proposed change, I say let's go for it.

[krinsman]

@tslaton This sounds right to me -- I added the last ?next=... part mostly because I didn't remember how all of the hub redirects work, and so copying it from the nameless server case seemed the safest/most conservative thing.

Have you added/merged those changes into the PR?

This page seems to explain it the best, although doesn't really address the next query argument.
https://github.com/jupyterhub/jupyterhub/blob/071e375d5fd038e70c29c619d199bf2c9c090d5c/docs/source/reference/urls.md#hubuserusernameservername
Probably a relevant quote for this issue:

Prior to 1.0, this URL (/hub/user/:username[/:servername]) itself was responsible for spawning servers, and served the progress page if it was pending, redirected to running servers, and This was useful because it made sure that requested servers were restarted after they stopped, but could also be harmful because unused servers would continuously be restarted if e.g. an idle JupyterLab frontend were open pointed at it, which constantly makes polling requests.
Special handling of API requests
Requests to /user/:username[/:servername]/api/... are assumed to be from applications connected to stopped servers. These are failed with 503 and an informative JSON error message indicating how to spawn the server. This is meant to help applications such as JupyterLab that are connected to a server that has stopped.
Version changed: 1.0
JupyterHub 0.9 failed these API requests with status 404, but 1.0 uses 503.

EDIT: Actually probably the more relevant portion of the page for our case:
https://github.com/jupyterhub/jupyterhub/blob/071e375d5fd038e70c29c619d199bf2c9c090d5c/docs/source/reference/urls.md#hubspawnusernameservername

/hub/spawn[/:username[/:servername]]
Requesting /hub/spawn will spawn the default server for the current user. If username and optionally servername are specified, then the specified server for the specified user will be spawned. Once spawn has been requested, the browser is redirected to /hub/spawn-pending/....
If Spawner.options_form is used, this will render a form, and a POST request will trigger the actual spawn and redirect.
Version added: 1.0
1.0 adds the ability to specify username and servername. Prior to 1.0, only /hub/spawn was recognized for the default server.
Version changed: 1.0
Prior to 1.0, this page redirected back to /hub/user/:username, which was responsible for triggering spawn and rendering progress, etc.

I think the big takeaway here would be that the routing/redirects actually are substantially different between pre- and post-1.0, so it's understandable why this has been more difficult than expected. (At least I personally had incorrectly assumed that they were largely the same.)

[tslaton]

Case 1: Restarting a named server with ?next specified as above opens the "spawn pending" page in a new tab, which redirects to the hub home (as specified with ?next) when spawning is finished. At the hub home, the restarted named server is visible (as would be any other named servers on that hub) and clicking on the named server gets into JupyterLab.

I had been getting a 404 error attempting this previously, but that was due to an extra / at the end of the url before the ?next portion. It's easy to remedy.

Case 2: Restarting a named server without ?next as specified above opens the "spawn pending" page in a new tab, which shows the JupyterLab instance corresponding to the restarted named server immediately when spawning is finished (no need to click into it from the hub home).

I think the second case is the behavior I'd expect to see.

EDIT: The changes required to the PR if we desire Case 1 behavior are simply removing an extra / in the restartUrl. The changes required if we desire Case 2 behavior are dropping the whole ?next portion from the restartUrl. I can add the appropriate changes once we've settled on which of the two behaviors is desired. Although I expect Case 2 personally, it sounds like @blink1073 expects Case 1. Case 1 is also most consistent with the fallback url. We should probably be consistent about redirecting to the hub home in the named servers case and in the fallback case, regardless of which one is chosen.

[blink1073]

I think it is fair to have different behavior between pre-1.0 and post-1.0. I actually agree with Case 2 as the post-1.0 behavior. I am restarting this named server when I hit restart. Otherwise I'd log out and start a different named server.

[tslaton]

Unless the failing tests pose an issue (not sure they're related to this PR), it should be ready now. It follows the Case 2 behavior: when restarting a named server, the user will not be redirected to the hub home.

[blink1073]

LGTM, thanks!

[blink1073]

@meeseeksdev please backport to 1.0.x

[blink1073]

@meeseeksdev backport to 1.0.x