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

Add standalone mode to server #70

Merged
merged 1 commit into from
Aug 26, 2019
Merged

Conversation

Zsailer
Copy link
Member

@Zsailer Zsailer commented Aug 3, 2019

This is one way to implement "standalone" mode #64.

This adds standalone mode to the ServerApp itself (not just extensions). Extensions can then run a standalone server by exposing a flag that sets ServerApp.standalone=True.

Copy link
Contributor

@blink1073 blink1073 left a comment

Choose a reason for hiding this comment

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

LGTM!

@Zsailer
Copy link
Member Author

Zsailer commented Aug 9, 2019

@SylvainCorlay and @maartenbreddels, does this address voila's use case?

@rolweber
Copy link
Contributor

The description of standalone as "Run the server without enabling extensions." is confusing if extensions can set that flag. Because if the flag is set, its description implies that extensions are not enabled, so they shouldn't be able to set the flag.

@vidartf
Copy link
Member

vidartf commented Aug 12, 2019

@rolweber Any suggestions for a less confusing description?

@Zsailer
Copy link
Member Author

Zsailer commented Aug 12, 2019

@rolweber right. Perhaps that is confusing.

Calling jupyter <extension_name> --standalone feels like you're running a single application with no extensions enabled. Of course, the main running application is an extension itself. That's challenging to explain in the description...

@vidartf
Copy link
Member

vidartf commented Aug 12, 2019

Perhaps "Run the server without enabling other extensions." (emphasis added)

Edit: I guess this still needs to work if using ServerApp from something other than a ExtensionApp?

@Zsailer
Copy link
Member Author

Zsailer commented Aug 12, 2019

I guess this still needs to work if using ServerApp from something other than a ExtensionApp?

@vidartf yes, but I haven't thought of a scenario where you'd run standalone outside the commands jupyter server --standalone or jupyter <my-extension> --standalone.

@kevin-bates
Copy link
Member

I agree that this is confusing. Could someone state the behaviors that occur given multiple extension apps are enabled? (Assuming there's some enablement step for extension apps?)

juptyer server
juptyer <extension_name>
jupyter server --standalone
juptyer <extension_name> --standalone

I also thought I saw somewhere where multiple extensions could be listed. Is that true?

jupyter server <extension1> <extension2>...

@Zsailer
Copy link
Member Author

Zsailer commented Aug 12, 2019

@kevin-bates

  • jupyter server: launches ServerApp and lands on the server's default_url. If extensions are enabled in configuration files, i.e. listed in jupyter_server_config.d folder or set in the jpserver_extensions trait in the jupyter_server_config.py|json file, they get loaded.
  • jupyter <extension-name> launches ServerApp, appends the named extension, and redirects to the extensions default_url. Other extensions are only enabled if they are listed in the configuration files mentioned above.
  • jupyter server --standalone launches ServerApp and ignores all extensions, even if they are listed in the configuration files above.
  • jupyter <my-extension> --standalone launches ServerApp, appends the named extension, and ignores all other extensions, even if they are listsed in the configuration files above.

jupyter server <extension1> <extension2>...

This example describes a scenario where no extensions are enable by your configuration setup, so you enable them by the command line. There is a way to do this currently, but it looks a bit uglier than your example above. It looks something like (I believe):

jupyter server --jpserver_extensions={'extension1': {'enabled': True}, 'extension2': {'enabled': True}}

@kevin-bates
Copy link
Member

Thanks Zach, this was extremely helpful (to me).

It seems like we could simplify things if we made extension usage more explicit such that jupyter <extension_name> implies only that extension is used. Then, by making 'enabled' default to True, the multi-extension command line could be reduced to:

jupyter server --jpserver_extensions={'extension1', 'extension2',...} or use a parameter name like extension_apps: jupyter server --extension_apps={...}

Since things like the gateways will, most likely, also be extensions, I'm not sure there's a need for jupyter server --standalone and then one could argue that --standalone would not be necessary.

"Side by side" mode would be accomplished by configuring multiple extensions and running jupyter server. Likewise, a gateway instance would be accomplished by jupyter <gateway-extension-name> or jupyter server with the single gateway extension configured.

Just for further clarification, what is the use case for jupyter server --standalone?

@Zsailer
Copy link
Member Author

Zsailer commented Aug 12, 2019

Just for further clarification, what is the use case for jupyter server --standalone?

I don't have a use-case—it was just problematic trying to set "standalone mode" from the extension's entrypoint. That's because the ServerApp has to be aware of the standalone mode (i.e. the logic is hardcoded in the ServerApp here). I just exposed that option in the ServerApp's traits...

Alternatively, we could hide this option behind a hidden attribute in ServerApp and make ExtensionApps change it under the hood if that would be less confusing.

@Zsailer
Copy link
Member Author

Zsailer commented Aug 12, 2019

It seems like we could simplify things if we made extension usage more explicit such that jupyter <extension_name> implies only that extension is used.

My main reason for not limiting extensions to this behavior was to allow commands like jupyter lab and jupyter notebook to remain unchanged after depending on jupyter server. For example, users would still be able to invoke the jupyter notebook command and get the same experience as before the migration (assuming we get configurable traitlet aliases pointing to the right places).

However, I'm fine with making this more strict if that's the direction we want to go...

Then, by making 'enabled' default to True, the multi-extension command line could be reduced to:

jupyter server --jpserver_extensions={'extension1', 'extension2',...} or use a parameter name like extension_apps: jupyter server --extension_apps={...}

Again, I think this makes total sense, but it breaks previous pattern. That said, switching to jupyter server might be the time to break these patterns for simpler options.

@kevin-bates
Copy link
Member

kevin-bates commented Aug 12, 2019

Thanks Zach - great stuff.

Do extensionApps have metadata (or could that be supported)? If so, perhaps an extension could indicate that multi-use is allowed (default = False). So Voila (and a Gateway) would indicate 'single-use', while Notebook and Lab would reflect 'multi-use=True'.

Then jupyter <extension_name> would check that <extension_name> is multi-use and, if so, load any other enabled and multi-use extensions (skipping enabled, single-use, extensions). If <extension_name> is single-use, it doesn't go any further.

We could then disallow jupyter server unless --extension_apps (or --jpserver_extensions) are also specified (or, of course --help or --help-all). Doing so would boil everything down to two cases (and no --standalone mode):

  • jupyter <extension_name>
    Loads extension. If extension is multi-use, also loads any other multi-use and enabled extensions

  • jupyter server --extension_apps={<extension_name1>, <extension_name2>, ...}
    Load specified extensions only. If multiple, all must be enabled for multi-use

This implies that the multi-use decision is that of the extension's author and not the admin deploying the server - which I think is the right location.

@blink1073
Copy link
Contributor

👍 to @kevin-bates' proposal.

@vidartf
Copy link
Member

vidartf commented Aug 12, 2019

skipping enabled, single-use, extensions

Would you ever enable such extensions in the first place though?

@kevin-bates
Copy link
Member

Would you ever enable such extensions in the first place though?

I wouldn't (at least intentionally). 😃 The server needs to handle that case though.
I would recommend a warning be logged for that case, but the server continue. The more explicit case, where extensions are listed on the command line, should result in a startup failure (when mixed or multiple single-uses are listed).

@vidartf
Copy link
Member

vidartf commented Aug 15, 2019

If so, perhaps an extension could indicate that multi-use is allowed (default = False).

I think there are two aspects here that need to be distinguished:

  • "Do not autoload other extensions when starting me as main app"
  • "Do not load me into other apps"

In many cases these will overlap, but I don't think we should make that into a general assumption.

Also, I had a look at implementing @kevin-bates proposal, and as far as I can tell, that would require refactoring the launch functions, as you are now using the configurables of the primary extension to initialize the server.

If the main problem with this is a confusing name/description of the flag, how about a flag on server called "auto-load-extensions" (default True)? Or just change the description of standalone to say "Run the server without automatically loading all enabled extensions"?

To cover the "Do not autoload me into other app" case, simply don't extend ExtensionApp (because it is then never intended to be used as an extension, only as an app). Or am I missing something here?

@vidartf
Copy link
Member

vidartf commented Aug 15, 2019

To cover the "Do not autoload me into other app" case, simply don't extend ExtensionApp

Or rather, don't expose load_jupyter_server_extension on the module.

@vidartf
Copy link
Member

vidartf commented Aug 15, 2019

as far as I can tell, that would require refactoring the launch functions

I looked a little bit farther, and found a workable solution. See #75.

@kevin-bates
Copy link
Member

Thanks for looking into this @vidartf - I was going to look at this once I had available cycles and had a suspicion it would fall into the "easier said than done" category. It's great that you've uncovered some hurdles already - thanks.

If the main problem with this is a confusing name/description of the flag, how about a flag on server called "auto-load-extensions" (default True)? Or just change the description of standalone to say "Run the server without automatically loading all enabled extensions"?

I felt there was both confusion and redundancy and that by some refactoring and "rule-setting" we could eliminate --standalone altogether. I also feel that the extension's author should state how the extension should be used, not the admin.

A couple comments...

I think there are two aspects here that need to be distinguished:

  • "Do not autoload other extensions when starting me as main app"
  • "Do not load me into other apps"

In many cases these will overlap, but I don't think we should make that into a general assumption.

Will it ever be the case that an extension that requires it be "standalone" ever allow itself to be loaded into other main applications? I find that hard to understand since the purpose of standalone is to expect exclusive use of the server irrespective of when/how started. So, I think you're right about the overlap, but I think we should say the overlap is 100%. Once standalone - always standalone.

When I read your heading to #75 I thought, "perfect, standalone extensions will just derive from a StandaloneApp which derives from ExtensionApp." I.e., StandaloneApp is just a special form of ExtensionApp and that provides the necessary metadata. It's an explicit action on the author's part to state intention for how their extension application is meant to be used.

But instead, #75 is just the opposite. All extension apps are StandaloneApps. This seems backwards to me and I suspect it's that way to support the non-overlapped case where a given StandaloneApp might also allow itself to be loaded as an extension. In my opinion, such extensions should only derive from ExtensionApp or provide two classes; one that derives from StandaloneApp that can only be used exclusively, and another that derives from ExtensionApp that can be loaded as a server extension. (Assuming the hierarchy is flipped such that StandaloneApp derives from ExtensionApp.) If the code between the two classes is 100% shared, cool, that's the author's call. However, I would think the author would just implement the ExtensionApp version and document that it is meant as a singleton primary.

Of course, this is probably another "easier said than done" thing, but from a high-level this makes more sense to me.

@SylvainCorlay
Copy link
Contributor

The current state and logic looks good to me. I will comment further on kevin's reservations when I am closer to a keyboard.

@Zsailer
Copy link
Member Author

Zsailer commented Aug 15, 2019

[from @vidartf] Also, I had a look at implementing @kevin-bates proposal, and as far as I can tell, that would require refactoring the launch functions, as you are now using the configurables of the primary extension to initialize the server.

This is exactly what I found, and this is what I was getting at earlier in the thread with this comment:

it was just problematic trying to set "standalone mode" from the extension's entrypoint. That's because the ServerApp has to be aware of the standalone mode (i.e. the logic is hardcoded in the ServerApp...

@kevin-bates brings up an interesting point about putting the "multi-extension" decision in the hands of the extension author. I hadn't thought about this PR from that perspective. I think I agree with it, though. This PR is really for extension authors, not users. As it's currently written, it actually makes things more confusing for users by offering too many ways to do the similar things. They don't really need this.

The original goal of this PR was to help extensions like Voila control this setting and implicitly prevent other extensions from being loaded for security reasons. Note, Voila could still be enabled next to other extensions using jupyter server --jpserver_extension={...} (we can devote efforts to make this command least characters). Otherwise, calling jupyter voila launches Voila alone.

This suggests to me that we shouldn't provide a --standalone flag, but instead, hardcode this setting as a hidden attribute inside ServerApp than can be changed by subclasses of ExtensionApp.

@vidartf
Copy link
Member

vidartf commented Aug 15, 2019

StandaloneApp is just a special form of ExtensionApp and that provides the necessary metadata.

Actually, StandaloneApp and ExtensionApp should probably be side-by-side with a common base, but StandaloneApp ended up not having any extra code, so I just put that as the base.

Will it ever be the case that an extension that requires it be "standalone" ever allow itself to be loaded into other main applications?

I don't think so, but I was thinking of the other corner case:

  • "Do not load me into other apps"
  • "Do auto load other extensions when running me as the main app"

This case would (in the lingo of my PR) be a StandaloneApp, but would pass load_extensions=True to the ServerApp.

@Zsailer
Copy link
Member Author

Zsailer commented Aug 15, 2019

See another possible solution in #76. 😆 This makes standalone mode an option set by extension developers.

This does not solve the issue of shortening the jupyter server --jpserver_extension... command. But it reduces the number of confusing options for users.

@rolweber rolweber merged commit ab003db into jupyter-server:master Aug 26, 2019
@Zsailer Zsailer deleted the standalone branch January 10, 2020 17:35
Zsailer pushed a commit to Zsailer/jupyter_server that referenced this pull request Nov 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants