Standalone mode set by extension authors #76

Zsailer · 2019-08-15T15:59:43Z

My alternative solution to #70 and #75.

This makes standalone mode an option only set by extension authors, not a configurable trait that users can set...

vidartf · 2019-08-15T16:08:28Z

I thought that adding load_extensions=True as a keyword arg to ServerApp.initialize was pretty elegant (as this is the only place it is currently used). It also avoids the smell of setting _ variables on another object.

-    def initialize(self, argv=None):
+    def initialize(self, argv=None, load_extensions=True):
        super(ServerApp, self).initialize(argv)
        self.init_logging()
...
        self.init_signal()
-       self.init_server_extensions()
+       if load_extensions is True:
+           self.init_server_extensions()

Zsailer · 2019-08-15T16:15:33Z

That is pretty elegant 😃

Is there any danger to changing initialize's signature? That's what made me hesitate using this solution in the original PR.

kevin-bates · 2019-08-15T16:28:47Z

I guess I still prefer an empty subclass StandaloneApp ...

class StandaloneApp(ExtensionApp):
    pass

then use...

if not isinstance(self, StandaloneApp):
     self.init_server_extensions()

Then no _ variables and self is always available wherever these checks are necessary and it provides a location for future divergence/overrides. For example, StandaloneApp could simply define init_server_extensions() as a no-op (allowing the conditional to be removed).

SylvainCorlay · 2019-08-15T17:01:00Z

In the case of two serverapp extensions (voila and lab)

when launching jupyter voila --standalone (or without standalone if voila is a standalone app), I expect jupyterlab to not be served.
when launching jupyter lab, I expect to be able to use voila (especially for the voila-preview jupyterlab extension).

kevin-bates · 2019-08-15T17:34:44Z

Thanks @SylvainCorlay - that's interesting and great example.

It seems like in the latter case, voila is merely a server extension and not an application and it would be nice that that be expressed in a class hierarchy somehow. Some StandaloneApps are also extensions and some are not. All ExtensionApps are also extensions by default. Is there a way to express that?

I apologize for dragging this on. Perhaps this is a matter of clarifying the jupyter server command itself. Perhaps we should require jupyter server be the use-case for a headless server so that it not load any extension apps or it not be a valid command at all - producing output that an extensionApp is required.

We could then remove --standalone by having a StandaloneApp(ExtensionApp) class, but allow StandaloneApps to be loaded as extensions - assuming there's a way to distinguish between a "primary" extensionApp (i.e., the one specified on the CLI) vs. a "secondary" extensionApp (loaded by the primary). I suppose this raises the question - how does an author state that they don't want their StandaloneApp loaded as an extension?

Can an "app" distinguish between it being primary vs. secondary - both at load and runtime? I think that would be useful.

SylvainCorlay · 2019-08-15T18:08:31Z

It seems like in the latter case, voila is merely a server extension and not an application and it would be nice that that be expressed in a class hierarchy somehow. Some StandaloneApps are also extensions and some are not. All ExtensionApps are also extensions by default. Is there a way to express that?

It does not seem to me that there is really a distinction. I was merely assuming the voila command would be an alias for jupyter voila --standalone.

vidartf · 2019-08-15T18:21:20Z

Given that we could possibly want all 4 combinations of "autoload me/not" × "autoload others/not", I believe the server app having a flag "load other extensions/not" should be sufficient (#76). Apps that do not want to be used as extensions should then not expose the server extension "magics" in the module.

Zsailer · 2019-08-15T18:23:09Z

@vidartf Did you mean to link #70 (instead of #76)?

vidartf · 2019-08-15T19:02:06Z

No, I was confused about where I was posting, not where I was linking 😀

vidartf

If we can pass flag instead of writing private var (without breaking anything), that would be better, but I think this concept is currently the best one.

Zsailer · 2019-08-15T22:14:34Z

@vidartf Updated this PR to use #75's way of allowing extensions to prevent other extensions from being loaded.

kevin-bates · 2019-08-15T22:33:38Z

jupyter_server/serverapp.py

@@ -1469,7 +1469,8 @@ def initialize(self, argv=None):
        self.init_webapp()
        self.init_terminals()
        self.init_signal()
-        self.init_server_extensions()
+        if load_extensions:
+            self.init_server_extensions()


I think it might be a good idea to log an informational message when load_extensions is False stating that "<extension_name> is running in exclusive mode" (or whatever terminology is appropriate). This way any confusion as to why other extensions aren't appearing is more clear.

Likewise, I think a debug message in init_server_extensions() stating which extension is being loaded would also be helpful information. This way admins can confirm their configurations are behaving as expected.

Added some logging messages.

kevin-bates

Another comment for now. (Not sure if duplicates were created - on a bouncy bus at the moment - I apologize if that was the case.)

kevin-bates · 2019-08-15T23:08:25Z

jupyter_server/serverapp.py

@@ -1415,6 +1415,8 @@ def init_server_extensions(self):
                    func = getattr(mod, 'load_jupyter_server_extension', None)
                    if func is not None:
                        func(self)
+                    # Add debug log for loaded extensions.
+                    self.log.debug("%s is enabled and loaded.", modulename))


These should be indented.

We should probably add an else: clause that logs a warning stating that the extension, while enabled, doesn't have a load_jupyter_server_extension() method.

Good catch!

👍 Warning added.

Zsailer · 2019-08-16T00:06:50Z

jupyter_server/extension/application.py

+            load_other_extensions=cls.load_other_extensions
+        )
+        # Log if extension is blocking other extensions from loading.
+        if cls.load_other_extensions:


@kevin-bates Here's the other logging message.

Thanks Zach. I'm wondering if the phrase "is running without loading other extensions." could get construed as a problematic symptom. That is, admins may not necessarily know that a given "primary" extension is configured for exclusive use and might wonder - why is it not loading other extensions? Just thinking that a phrase like the following might be more clear:

"{ext_name} is configured for exclusive use - no other extensions will be loaded."

This way, the intention is clear and not as ambiguous.

@Zsailer - I just noticed that the polarity on the condition is not correct. This should only occur if load_other_extensions is False.

Also, I'm thinking it might be better to move this log statement into ServerApp.initialize() as an else condition to where the other extensions are being loaded - or is that at too low a level?

Oh my. Good catch and sorry for the sloppiness! 🤦‍♂

Also, I'm thinking it might be better to move this log statement into ServerApp.initialize() as an else condition to where the other extensions are being loaded - or is that at too low a level?

My hesitation here is that we'd need to add another argument, extension_name, to ServerApp.initialize(). Right now, the ServerApp is completely unaware of ExtensionApps. Adding this argument would break this pattern. Do we think that's ok?

Ah - that makes sense - good point. Let's keep that pattern in tact as long as possible.

"{ext_name} is configured for exclusive use - no other extensions will be loaded."

I think the word "configured" might be confusing since it's not configurable by the user.

How about:

"jupyter {ext_name}" runs without loading other extensions. If you would like to run {ext_name} with other extensions, try runnig "jupyter server" with {ext_name} enabled.

rolweber · 2019-08-16T05:41:07Z

I'm in favor of flags over inheritance. This allows for extensions that can act standalone as well as cooperative. There's nothing wrong with having a convenience class that sets the flag by default. But the behavior should be driven by the flag, not by the class hierarchy.

kevin-bates · 2019-08-16T16:30:03Z

@rolweber - good point. The dual-mode behavior of Voila that @Sylvain stated put a damper on a class-based approach since StandaloneApps could then also be cooperative - which makes sense.

I think we've boiled this down to two primary ways to start the server, each with a couple semantic differences: jupyter <extension_app> and jupyter server and no --standalone. 🎉

Here are some concrete examples that should illustrate the differences ... (I've also used "footnotes" for questions.)

jupyter voila - Voila extends ExtensionApp and because it sets load_other_extensions = False, no other server extensions [1] will be loaded.
jupyter lab - Lab extends ExtensionApp and leaves load_other_extensions = True. As a result, any configured server extensions will be loaded - including Voila.[2,3]
jupyter server - Looks in the configuration files for ServerApp.jpserver_extensions and loads any enabled extensions. [4]
jupyter server --jpserver_extensions={...} - This is really just a form of the previous where the CLI is going to override anything found in the config files. Just stating it here for completeness. It would be cool - if possible - if use of the launch in this manner implies that all listed extensions are enabled. I guess that goes back to making the enabled boolean default to True.

[1]: By "server extension" are we (now) always talking about applications that extend ExtensionApp? That is, Voila could define its own configurable containing a list of server extensions (that do not derive from ExtensionApp and only apply to Voila) that are loaded by Voila directly - correct?

[2]: Is there a way for an ExtensionApp to say that it doesn't want to be 'cooperative'? I.e., extension authors require "complete exclusivity". Is allowing such a thing necessary?

[3]: Can a given ExtensionApp determine in what context it was loaded? Is that necessary? This probably only matters in the cases where load_other_extensions = False since those apps may want to change some behavior depending on how they're being used. This would also be a way for an author to enforce the "complete exclusivity" requirement and say "I don't want to participate" - converting its load function into a no-op in that case.

[4]: What should the behavior be if no extension apps are configured and enabled? Currently, it runs but I'm not sure how useful that is. However, I suppose this could be a way to validate a minimum configuration in times of troubleshooting, adding server extensions at each step - so I guess I'd be inclined to leave this alone.

vidartf · 2019-08-17T23:49:53Z

Is there a way for an ExtensionApp to say that it doesn't want to be 'cooperative'? I.e., extension authors require "complete exclusivity". Is allowing such a thing necessary?

One clear step would be to not define the server extension interfaces in the package (server extension path function, server extension load function), so that it cannot be used as an extension.

Another clear step would be to name all the app classes with a leading underscore to hint that "this class is not meant for reuse".

I don't think we should do anything from this package's side.

SylvainCorlay · 2019-08-18T17:29:53Z

[1]: By "server extension" are we (now) always talking about applications that extend ExtensionApp? That is, Voila could define its own configurable containing a list of server extensions (that do not derive from ExtensionApp and only apply to Voila) that are loaded by Voila directly - correct?

It was my understanding that by server extension we don't only talk about ExtensionApps. Basically we start creating server extensions as soon as we need new tornado entry points.

ExtensionApps are the subset of server extensions that has a default URL that we may want to start with a python entry point.

[2]: Is there a way for an ExtensionApp to say that it doesn't want to be 'cooperative'? I.e., extension authors require "complete exclusivity". Is allowing such a thing necessary?

Good question. I don't see a usecase for that at the moment though.

The reason why I brought up the standalone option for voilà is that it is sometimes important that we don't serve the filesystem or that we don't allow arbitrary code execution by giving a notebook interface. I expect the voila command-line to become an alias on jupyter voila --standalone, also specifying the allowed jupyter messages to disallow execution requests.

[3]: Can a given ExtensionApp determine in what context it was loaded? Is that necessary? This probably only matters in the cases where load_other_extensions = False since those apps may want to change some behavior depending on how they're being used. This would also be a way for an author to enforce the "complete exclusivity" requirement and say "I don't want to participate" - converting its load function into a no-op in that case.

It may be useful to e.g. disallow execution requests in the case of voilà, although since we should have the previously described alias, it is probably fine as-is.

[4]: What should the behavior be if no extension apps are configured and enabled?

I presume that the default start URL should be some jupyter equivalent to Apache's "it works" and some documentation about how to install extension apps.

When extension apps are enabled, they could be listed on that page.

kevin-bates · 2019-08-19T00:13:11Z

Thank you @SylvainCorlay - that's helpful.

Zsailer · 2019-08-23T17:02:16Z

Any reason not to merge this?

rolweber

lgtm

Zsailer added 2 commits August 7, 2019 14:38

allow server standalone mode

ab003db

make standalone mode a hidden attribute and not configurable

fd621c4

Zsailer mentioned this pull request Aug 15, 2019

Add standalone mode to server #70

Merged

vidartf approved these changes Aug 15, 2019

View reviewed changes

make load_extensions an arg to ServerApp

5bc87f1

fix instance argument in a static method

e416c74

kevin-bates reviewed Aug 15, 2019

View reviewed changes

Zsailer force-pushed the standalone2 branch from 4f6e7d2 to 6e4495b Compare August 15, 2019 23:05

kevin-bates reviewed Aug 15, 2019

View reviewed changes

add logging messages

d7b70e7

Zsailer force-pushed the standalone2 branch from 6e4495b to d7b70e7 Compare August 15, 2019 23:25

add warning if no extension funciton is found

89c184c

Zsailer force-pushed the standalone2 branch from fd0bae7 to 89c184c Compare August 16, 2019 00:05

Zsailer commented Aug 16, 2019

View reviewed changes

flip conditional statement bug

9e4448e

Zsailer mentioned this pull request Aug 23, 2019

[WIP] Voila as an ExtensionApp voila-dashboards/voila#270

Closed

rolweber approved these changes Aug 26, 2019

View reviewed changes

rolweber merged commit 37afd57 into jupyter-server:master Aug 26, 2019

Zsailer mentioned this pull request Aug 26, 2019

[ExtensionApp] "standalone" mode #64

Closed

Zsailer deleted the standalone2 branch January 10, 2020 17:35

Zsailer pushed a commit to Zsailer/jupyter_server that referenced this pull request Nov 18, 2022

Clean up publish (jupyter-server#76)

ebea785

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Standalone mode set by extension authors #76

Standalone mode set by extension authors #76

Zsailer commented Aug 15, 2019

vidartf commented Aug 15, 2019

Zsailer commented Aug 15, 2019

kevin-bates commented Aug 15, 2019 •

edited

Loading

SylvainCorlay commented Aug 15, 2019

kevin-bates commented Aug 15, 2019

SylvainCorlay commented Aug 15, 2019

vidartf commented Aug 15, 2019

Zsailer commented Aug 15, 2019

vidartf commented Aug 15, 2019 •

edited

Loading

vidartf left a comment

Zsailer commented Aug 15, 2019

kevin-bates Aug 15, 2019

Zsailer Aug 15, 2019

kevin-bates left a comment

kevin-bates Aug 15, 2019

Zsailer Aug 15, 2019

Zsailer Aug 16, 2019

kevin-bates Aug 16, 2019

kevin-bates Aug 16, 2019

Zsailer Aug 16, 2019

Zsailer Aug 16, 2019

kevin-bates Aug 16, 2019

Zsailer Aug 16, 2019 •

edited

Loading

rolweber commented Aug 16, 2019 •

edited

Loading

kevin-bates commented Aug 16, 2019 •

edited

Loading

vidartf commented Aug 17, 2019

SylvainCorlay commented Aug 18, 2019

kevin-bates commented Aug 19, 2019

Zsailer commented Aug 23, 2019

rolweber left a comment

Standalone mode set by extension authors #76

Standalone mode set by extension authors #76

Conversation

Zsailer commented Aug 15, 2019

vidartf commented Aug 15, 2019

Zsailer commented Aug 15, 2019

kevin-bates commented Aug 15, 2019 • edited Loading

SylvainCorlay commented Aug 15, 2019

kevin-bates commented Aug 15, 2019

SylvainCorlay commented Aug 15, 2019

vidartf commented Aug 15, 2019

Zsailer commented Aug 15, 2019

vidartf commented Aug 15, 2019 • edited Loading

vidartf left a comment

Choose a reason for hiding this comment

Zsailer commented Aug 15, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kevin-bates left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Zsailer Aug 16, 2019 • edited Loading

Choose a reason for hiding this comment

rolweber commented Aug 16, 2019 • edited Loading

kevin-bates commented Aug 16, 2019 • edited Loading

vidartf commented Aug 17, 2019

SylvainCorlay commented Aug 18, 2019

kevin-bates commented Aug 19, 2019

Zsailer commented Aug 23, 2019

rolweber left a comment

Choose a reason for hiding this comment

kevin-bates commented Aug 15, 2019 •

edited

Loading

vidartf commented Aug 15, 2019 •

edited

Loading

Zsailer Aug 16, 2019 •

edited

Loading

rolweber commented Aug 16, 2019 •

edited

Loading

kevin-bates commented Aug 16, 2019 •

edited

Loading