Make the default drive in the Contents Service configurable

[Zsailer]

References

Fixes #16099.

Code changes

This PR makes two changes:

• always creates a drive named "contents" to connect the server REST API
• defaults to making this drive the defaultDrive. The default drive doesn't need a prefix when a file path is given.
• adds a method, setDefaultDrive to update the default drive.

Adds a public API to set/change the defaultDrive in the contents service.

Leaving this in draft state for now to discuss.

User-facing changes

There are no user-facing changes by default.

This allows extensions like jupyter-collaboration to set the default drive to something other than the REST API. In the RTC case, it uses the ydoc APIs.

Backwards-incompatible changes

None.

@martinRenou @SylvainCorlay @davidbrochart @dlqqq @jzhang20133 @jtpio

[jupyterlab-probot]

Thanks for making a pull request to jupyterlab!
To try out this branch on binder, follow this link:

[krassowski]

I guess this is intended to fix #16099 (missing 9).

[Zsailer]

oops! haha yes, thank you @krassowski. Updated.

[jtpio]

Thanks @Zsailer for looking into this 👍

Adds a public API to set/change the defaultDrive in the contents service.

It looks like being able to set a different default drive would be similar to being able to set other services (for example the kernel manager) provided by the ServiceManager by default as well (#15329).

If we start adding new public methods to replace parts of a ServiceManager, there may be a chance the API surface area will increase in the future, if we want to allow for swapping other services?

Raising the question now so we can get a better idea of how this would play with #15329 in the long run.

[Zsailer]

It looks like being able to set a different default drive would be similar to being able to set other services (for example the kernel manager) provided by the ServiceManager by default as well (#15329).

I think this particular case is slightly different. This doesn't replace the e.g. Contents Manager, simply changing a property on the Contents Manager, so it's much less disruptive than swapping out a whole service.

It does increase the API surface a small amount. However, regardless of what we do to improve the pluggability of the ServiceManager proposed in #15329, swapping out default Drives underneath the Contents API seems like something we'd want to allow either way.

[fcollonval]

I think allowing to change the default drive at runtime is a bad idea because there are no control over when it changes and it can be changed at any moment by multiple extensions.

As the default drive is already a top options of the ServiceManager:

https://github.com/jupyterlab/jupyterlab/blob/main/packages/services/src/manager.ts#L223

I second the idea of @jtpio that a better approach would be to make the service manager more extensible with a clear definition of the default plugins used.

Something like:

Resolve plugins for the service manager -> Create the service manager -> Create the app -> load the other plugins.

[fcollonval]

The default drive name is Default

https://github.com/jupyterlab/jupyterlab/blob/main/packages/services/src/contents/index.ts#L1079

So although it is not a good name if this PR is merged, I would not change it for backward compatibility.

[Zsailer]

👍

[Zsailer]

I think allowing to change the default drive at runtime is a bad idea because there are no control over when it changes and it can be changed at any moment by multiple extensions.

Fair enough.

Resolve plugins for the service manager -> Create the service manager -> Create the app -> load the other plugins.

Thanks @fcollonval. We discussed this in the JupyterLab call today and reached a similar conclusion; however, the specifics for how to achieve this was a bit muddy. I'll think about this more and see if I can come up with a draft PR.

@afshin previously attempted something similar in #13558. I'll dive into this a bit more and see what I can learn.

Thank you, all, for reviewing so quickly. I'll post an update here once I can get more time to think about this.

[fcollonval]

Hey @Zsailer sorry I was not able to connect to the call. I have a pretty good idea of how to achieve this. But unfortunately it requires a change in the lumino Application (for the greater good).

Basically it involves extracting the plugin resolver logic in its own class (because why should such great feat be hard linked to the Application). Once we get that the logic in index.js will be:

1. Create the plugin resolver with all the plugins
2. Provide that resolver to JupyterLab - by inheritance that passes to the JupyterFrontEnd and finally to the lumino Application
3. In JupyterFrontEnd, use as default service manager, the one directly provided (for backward compatibility), or the one from a new token ServiceManager.IManager or fallback to create a new ServiceManager.
4. Deprecate the access to the service manager from the app (see more details below) and prefer requesting the token in the plugins.

I think the first PR should stop there (aka not make the ServiceManager a composition of multiple tokens) as that can be addressed in a follow-up PR.

There are also some security concerns raised by @echarles; should we have a specific control on the ServiceManager provider? As mentioned by @afshin in #13558 (comment) I don't think this is really an issue as the service manager can today be replaced at runtime. In fact the above proposal can even improve security because the previous hack won't be possible any longer if the only access to the service manager is through the token (not happening before a major release). And which extensions can be installed could be enforced through the existing white/black lists.

[fcollonval]

Food for thoughts, for me the Application in Lumino is a mistake hurting the plugin based approach of Lumino. Going full plugins even for command registry, shell, context menu, etc... would be a much greater pattern with no need to provide the app but only the required dependencies to all plugins.

[jtpio]

Basically it involves extracting the plugin resolver logic in its own class (because why should such great feat be hard linked to the Application)

@fcollonval should we discuss this in an issue on the Lumino repo? Or maybe there is one already?

[fcollonval]

@fcollonval should we discuss this in an issue on the Lumino repo? Or maybe there is one already?

Issue opened => jupyterlab/lumino#697

[jtpio]

Issue opened => jupyterlab/lumino#697

Thanks!

I was not able to connect to the call

For reference there is the link to the recording in the meeting notes of the call, in case that can be useful: jupyterlab/frontends-team-compass#229 (comment)