-
Notifications
You must be signed in to change notification settings - Fork 308
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
@SylvainCorlay and @maartenbreddels, does this address voila's use case? |
The description of |
@rolweber Any suggestions for a less confusing description? |
@rolweber right. Perhaps that is confusing. Calling |
Edit: I guess this still needs to work if using ServerApp from something other than a |
@vidartf yes, but I haven't thought of a scenario where you'd run standalone outside the commands |
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?)
I also thought I saw somewhere where multiple extensions could be listed. Is that true?
|
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):
|
Thanks Zach, this was extremely helpful (to me). It seems like we could simplify things if we made extension usage more explicit such that
Since things like the gateways will, most likely, also be extensions, I'm not sure there's a need for "Side by side" mode would be accomplished by configuring multiple extensions and running Just for further clarification, what is the use case for |
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 Alternatively, we could hide this option behind a hidden attribute in ServerApp and make |
My main reason for not limiting extensions to this behavior was to allow commands like However, I'm fine with making this more strict if that's the direction we want to go...
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. |
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 We could then disallow
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. |
👍 to @kevin-bates' proposal. |
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 think there are two aspects here that need to be distinguished:
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 To cover the "Do not autoload me into other app" case, simply don't extend |
Or rather, don't expose |
I looked a little bit farther, and found a workable solution. See #75. |
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.
I felt there was both confusion and redundancy and that by some refactoring and "rule-setting" we could eliminate A couple comments...
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 But instead, #75 is just the opposite. All extension apps are Of course, this is probably another "easier said than done" thing, but from a high-level this makes more sense to me. |
The current state and logic looks good to me. I will comment further on kevin's reservations when I am closer to a keyboard. |
This is exactly what I found, and this is what I was getting at earlier in the thread with this comment:
@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 This suggests to me that we shouldn't provide a |
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.
I don't think so, but I was thinking of the other corner case:
This case would (in the lingo of my PR) be a |
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 |
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
.