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

extend/override connection lost for jupyterhub #6176

Closed
minrk opened this issue Apr 9, 2019 · 13 comments · Fixed by #6399
Closed

extend/override connection lost for jupyterhub #6176

minrk opened this issue Apr 9, 2019 · 13 comments · Fixed by #6399

Comments

@minrk
Copy link
Contributor

@minrk minrk commented Apr 9, 2019

In JupyterHub, there's a typical cause for "Connection Lost" (server stopped) and an action to take (go here to restart it). It would be useful to be able to customize the Connection Lost event when running with JupyterHub to point to the resolution, rather than the generic message that doesn't know how to restart the server. Does such a mechanism already exist?

cf jupyterhub/jupyterlab-hub#84

@jasongrout jasongrout added this to the 1.0 milestone Apr 17, 2019
@jasongrout jasongrout self-assigned this Apr 17, 2019
@jasongrout
Copy link
Contributor

@jasongrout jasongrout commented May 21, 2019

@minrk, is this the dialog you are trying to override?

let title = 'Server Connection Error';
let networkMsg =
'A connection to the Jupyter server could not be established.\n' +
'JupyterLab will continue trying to reconnect.\n' +
'Check your network connection or Jupyter server configuration.\n';

@ian-r-rose ian-r-rose self-assigned this May 22, 2019
@jasongrout jasongrout removed their assignment May 23, 2019
@vidartf
Copy link
Member

@vidartf vidartf commented May 24, 2019

What would we say would be the recommended steps for juyterlab-hub to use the extension point from #6399? Would their install instructions need to include a step to disable the core plugin, or is this something that can be configured via a deploy step?

@ian-r-rose
Copy link
Member

@ian-r-rose ian-r-rose commented May 24, 2019

I think that extensions which override default tokens take precedence, though maybe we should be more explicit than that. @afshin can you comment on the resolution order of tokens?

@vidartf
Copy link
Member

@vidartf vidartf commented May 28, 2019

As far as I can read from the phosphor code, trying to re-register a token will throw an exception. In Lab, this seems to be side-stepped such that "first come" will be used, and subsequent registrations will have their errors suppressed, but stored:

registerPluginModule(mod: JupyterLab.IPluginModule): void {
let data = mod.default;
// Handle commonjs exports.
if (!mod.hasOwnProperty('__esModule')) {
data = mod as any;
}
if (!Array.isArray(data)) {
data = [data];
}
data.forEach(item => {
try {
this.registerPlugin(item);
} catch (error) {
this.registerPluginErrors.push(error);
}
});
}

I'm not sure if I understand in which order extensions are loaded.

@ian-r-rose
Copy link
Member

@ian-r-rose ian-r-rose commented May 28, 2019

Re-registering the same id will throw an error, but providing an overriding token is okay, provided it has a different ID.

@ian-r-rose
Copy link
Member

@ian-r-rose ian-r-rose commented May 28, 2019

See, for instance, this note.

@vidartf
Copy link
Member

@vidartf vidartf commented May 28, 2019

I'm confused, won't jupyter hub have to use the same token?

@vidartf
Copy link
Member

@vidartf vidartf commented May 28, 2019

Ah, I see now, plugin ID is different from token of course. Phosphor uses last-write wins logic. I'm still missing the puzzle piece about module sort order to get the full picture though.

image

@ian-r-rose
Copy link
Member

@ian-r-rose ian-r-rose commented May 28, 2019

There is a distinction between token and plugin ids. It will provide the same token, and I think that is okay.

@ian-r-rose
Copy link
Member

@ian-r-rose ian-r-rose commented May 28, 2019

Yeah, it's was my impression that extensions are loaded later, If that is not the case, I think we should make it the case.

@vidartf
Copy link
Member

@vidartf vidartf commented May 28, 2019

I think all extensions are either sorted alphabetically or in random order.

Still, I am now thinking that maybe it would have been cleaner to implement this extension point as a method on app. There might have been some cons for that approach though?

@vidartf
Copy link
Member

@vidartf vidartf commented May 29, 2019

I opened #6434 for a more constructive suggestion for this.

@lock
Copy link

@lock lock bot commented Aug 6, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related discussion.

@lock lock bot locked as resolved and limited conversation to collaborators Aug 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

4 participants