Allow customizing the `ServiceManager` with plugins #16794

jtpio · 2024-09-19T12:04:03Z

References

Fixes #15329
Fixes #12504
Replaces #16556
Replaces #16783
Closes #16141
Depends on #16804

More references:

Based on Extract plugin handling in a PluginRegistry class independent of the Application lumino#703
Inspired by Allow the application service manager to be set by a plugin #13558
Alternative to Add content providers #16744
Follows a similar approach as in JupyterLite, which registers "server" plugins separately on a different Lumino application (the plugin registry was not usable outside an application before Extract plugin handling in a PluginRegistry class independent of the Application lumino#703): https://github.com/jupyterlite/jupyterlite/blob/eb0bcb543703213ea883dc1da948ce07494a3148/packages/server-extension/src/index.ts

Motivation

Code changes

This can be tested using this example extension, which disables @jupyterlab/application-extension:default-drive and replaces with its own implementation: https://github.com/jtpio/jupyterlab-custom-service-manager-plugins

Example providing a custom default drive

Example providing a custom ContentsManager that logs each get call

import {
  Contents,
  ContentsManager,
  IContentsManager,
  ServiceManagerPlugin
} from '@jupyterlab/services';

class CustomContents extends ContentsManager {
  async get(
    path: string,
    options?: Contents.IFetchOptions
  ): Promise<Contents.IModel> {
    console.log('CustomContents.get', path);
    return super.get(path, options);
  }
}

const contentsPlugin: ServiceManagerPlugin<Contents.IManager> = {
  id: 'myextension:contents-drive',
  description: 'A JupyterLab extension providing a custom contents manager',
  autoStart: true,
  provides: IContentsManager,
  activate: (_: null): Contents.IManager => {
    console.log('Using a CUSTOM contents manager plugin');
    return new CustomContents();
  }
};

export default [
  contentsPlugin,
];

User-facing changes

None

Backwards-incompatible changes

None

jupyterlab-probot · 2024-09-19T12:04:06Z

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

afshin · 2025-02-05T10:02:50Z

The office hours exist for members of the community to come ask questions from members of the EC. If the question requires following up or further decisions to be made, then normal procedure will follow.

From my point of view, this is a case of a decision that should be made within the Jupyter Frontends Council by the members of this subproject. I imagine if you come to office hours, you might make the case that this particular technical decision on whether to merge a specific feature-related PR in the JupyterLab code base should be made by the EC. I don't agree with this interpretation, but you're obviously free to make that case tomorrow.

Nonetheless, I again strongly encourage you to come today to the appropriate meeting to discuss this and to make one final case after all this discussion. I don't believe EC office hours is the correct venue to talk about a JupyterLab PR.

afshin · 2025-02-05T10:13:32Z

@echarles, you stated:

In the meantime, I expect this PR to not be merged whatever result would come from the vote you have opened.

After months of discussion:

Not a single other member of the Jupyter Frontends Council has objected to merging this PR and a few have already approved it.
Not a single other member of the Jupyter Frontends Council has seconded your motion to put this PR to a vote.
The Jupyter Security Subproject has spent an entire meeting discussing whether your security claims have merit and they have decided this feature does not pose an objectionable risk.

You wish to appeal to a body (the Jupyter Executive Council) that does not adjudicate technical decisions made by subprojects. That is absolutely your right, but I don't think included in that right is the ability to yet again stall motion on a feature simply because you alone object to it.

echarles · 2025-02-05T10:14:43Z

The discussion has moved from technical to governance, the governance aspect is the topic I am willing to share with the executive council. I hope enough people will be there tomorrow, and in all case, I will request a follow-up so the case is shared with all the members of the executive council.

afshin · 2025-02-05T10:30:00Z

If you have governance-related questions, as you indicate, please feel free to raise them at the Executive Council's office hours tomorrow.

If you have any further conversation you'd like to have about this PR, please feel free to join today's call where this PR and its resolution will be a topic of discussion.

echarles · 2025-02-05T10:36:00Z

I can not join today call, and also diccussed 2 days ago during a call with @jtpio. I would like to say again tomorrow topic will be the invalidity of the vote, so I expect the JupyterLab council @jupyterlab/jupyterlab-council to continue discussing but not merge this PR until the governance point is treated by the Jupyter Executive council.

SylvainCorlay · 2025-02-05T10:43:48Z

so I expect the JupyterLab council @jupyterlab/jupyterlab-council to continue discussing but not merge this PR until the governance point is treated by the Jupyter Executive council.

I don't think this is a fair expectation at this point.

afshin · 2025-02-05T10:46:12Z

@echarles stated:

I would like to say again tomorrow topic will be the invalidity of the vote

There was no vote. I mistakenly asked people to vote in discussion thread. Four people did. Then Eric questioned whether voting with a secret ballot was even allowed (NB: it is). So I re-read the decision-making guide myself to see if there's anything against a secret ballot. There isn't.

But there is a requirement that someone else seconds your motion to call a vote. No one has. No one else has objected to merging this PR. No one else in the Frontends Council has recommended calling a vote. All the other indications (the votes that came in before I asked if anyone even supports holding such a vote and also the approvals on this PR) are positive.

(NB: David Q and Rick W did say that as members of the Security subproject, they think a vote on this would be okay. But they weren't saying that as voters in this body.)

So strictly speaking, you're right. That discussion vote was invalid. It was invalid because no one but Eric wanted to vote on it. And given the support it got in that poll and in this PR, I imagine that even if someone on the council did second the motion to hold a vote, the PR would still be accepted.

fcollonval · 2025-02-05T13:52:34Z

Hey all,

I will second @echarles to go for a public vote. I think it is important for heated subject that the decision should be clear and public to reflect the open-minded and transparent decision process of this community.
As @afshin pointed out, the discussion with the poll opened in the council repository was not meant to be a formal vote. It would be good to reboot the vote via an issue on the team compass repository with a ping to the council member asking to vote.

echarles · 2025-02-05T14:23:05Z

Thank you @fcollonval

@afshin I have opened a vote jupyterlab/frontends-team-compass#273

So I re-read the decision-making guide myself to see if there's anything against a secret ballot. There isn't.

@afshin to me there is. Let's discuss about that tomorrow.

@jupyterlab/jupyterlab-council Please vote on jupyterlab/frontends-team-compass#273 - you should have been tagged in the vote itself. Please also consider https://github.com/jupyterlab/council/discussions/28 as closed (if you don't know about that discussion, that is also fine and logical, as you have not been tagged on it, so quite hard to vote without even being invited)

cc/ Executive Council @afshin @Ruv7 @ellisonbg @fperez @jasongrout @Zsailer (as discussed, I will join tomorrow 06 Feb call).

Zsailer · 2025-02-05T22:07:48Z

Cross referencing a reply I made on the other thread, realizing I should have posted here: jupyterlab/frontends-team-compass#273 (comment)

The reason we need this PR is because I made a design decision, which I later came to regret, back in 2019 when I initially jupyterlab/jupyterlab#5845.
In this initial implementation, the API required all Jupyter frontends to ship with a serviceManager instead of making service managers plugins.

I have one anecdotal piece of evidence where the old ServiceManager design decision blocked me (sorry @afshin 😉).

I tried to create a new (RTC-enabled) Notebook-like application that gave JupyterLab user's a link to share to a single document in their server for (real-time) collaborative editing only (no kernel execution allowed). Invitees would get access to only that document, in a single document Notebookv7 view to collaborate on the document's content. The requirements were:

the UI was a stripped down Notebook v7, just used for editing the content of the notebook.

should only talk to the host's contents and y-doc websocket APIs.

No access to the host's kernels, kernelspecs, terminals, sessions, etc.

Still include many of the plugins, like TOC, ipywidgets, vega, jupyter-collaboration etc.

In the idea case, I would have just liked to use Notebook v7 with a configurable ServiceManager class to replace the parts I needed (note that I do need a "replacement" here, not just an API for addition). Instead, the hard-coded ServiceManager posed a problem for me; it hardcodes all Jupyter Server APIs and expects all of the services to working. It also automatically polls the kernelspecs APIs (any sessions?) on a regular interval. While we can use server auth to prevent access to these APIs, the client constantly dumps errors due to this polling behavior. On the other hand, there was no way to stop this unwanted behavior without creating a new app from scratch. Ultimately, this is what I did, and it led to a lot of extra duplicate logic.

Under the proposed PR, I would get what I needed much easier. There may be different approaches we could take, but this PR seems reasonable to me. Just making the current servicemanager "expandable" using providers doesn't feel quite enough and I haven't seen a strong argument on why we shouldn't other than perceived risk of API scope creep.

Just sharing one, personal use-case outside of just JupyterLite.

Zsailer · 2025-02-05T22:13:06Z

Maybe one improvement that would help address some of @echarles usability concerns is to consider some way to dump out the plugin dependency graph, so if a user reports some unexpected behavior because on conflicting plugins, we can more easily trace the issue?

echarles · 2025-02-06T05:57:12Z

cc/ Executive Council (as discussed, I will join tomorrow 06 Feb call).

A valid vote being now opened, I will thus not join today Executive council to discuss this case cc/ Executive Council @afshin @Ruv7 @ellisonbg @fperez @jasongrout @Zsailer

krassowski

I noted a tiny typo, otherwise good!

packages/application/src/lab.ts

krassowski · 2025-02-13T17:20:36Z

packages/application/src/lab.ts

    // Populate application info.
-    this._info = { ...JupyterLab.defaultInfo, ...info };
+    this._info = new JupyterLab.Info(options);


Note to self: this will prevent writing to some attributes, but it's ok because it is private

Co-authored-by: Michał Krassowski <5832902+krassowski@users.noreply.github.com>

jtpio · 2025-02-13T17:58:58Z

Thanks @krassowski for the review!

github-actions bot assigned jtpio Sep 19, 2024

echarles mentioned this pull request Feb 5, 2025

[VOTE] Make the ServiceManager Pluggable jupyterlab/frontends-team-compass#273

Closed

63 tasks

jtpio added 3 commits February 12, 2025 19:13

Merge branch 'main' into split-service-manager

23d99f5

fix typescript version

7bcf69d

Merge branch 'main' into split-service-manager

b85096e

krassowski approved these changes Feb 13, 2025

View reviewed changes

Apply suggestions from code review

b5f669e

Co-authored-by: Michał Krassowski <5832902+krassowski@users.noreply.github.com>

krassowski mentioned this pull request Feb 13, 2025

Make the default drive in the Contents Service configurable #16141

Closed

Merge branch 'main' into split-service-manager

e67c775

davidbrochart mentioned this pull request Feb 17, 2025

In JupyterLite deployment, 403 when renaming or deleting files geojupyter/jupytergis#468

Closed

Merge branch 'main' into split-service-manager

4272b6e

krassowski merged commit 183042e into jupyterlab:main Feb 19, 2025
82 of 83 checks passed

jtpio deleted the split-service-manager branch February 20, 2025 06:09

jtpio mentioned this pull request Mar 20, 2025

Use a PluginRegistry for loading plugins jupyter/notebook#7615

Closed

Uh oh!

Allow customizing the ServiceManager with plugins #16794

Allow customizing the ServiceManager with plugins #16794

Uh oh!

Conversation

jtpio commented Sep 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

References

Motivation

Code changes

User-facing changes

Backwards-incompatible changes

Uh oh!

jupyterlab-probot bot commented Sep 19, 2024

Uh oh!

afshin commented Feb 5, 2025

Uh oh!

afshin commented Feb 5, 2025

Uh oh!

echarles commented Feb 5, 2025

Uh oh!

afshin commented Feb 5, 2025

Uh oh!

echarles commented Feb 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SylvainCorlay commented Feb 5, 2025

Uh oh!

afshin commented Feb 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fcollonval commented Feb 5, 2025

Uh oh!

echarles commented Feb 5, 2025

Uh oh!

Zsailer commented Feb 5, 2025

Uh oh!

Zsailer commented Feb 5, 2025

Uh oh!

echarles commented Feb 6, 2025

Uh oh!

krassowski left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

krassowski Feb 13, 2025

Choose a reason for hiding this comment

Uh oh!

jtpio commented Feb 13, 2025

Uh oh!

Uh oh!

Uh oh!

Allow customizing the `ServiceManager` with plugins #16794

Allow customizing the `ServiceManager` with plugins #16794

jtpio commented Sep 19, 2024 •

edited

Loading

echarles commented Feb 5, 2025 •

edited

Loading

afshin commented Feb 5, 2025 •

edited

Loading