jupyterlab
jupyterlab copied to clipboard
Add events service
work in progress
This PR adds support for the newly added /api/events endpoints in jupyter_server
References
https://github.com/jupyter-server/jupyter_server/issues/780
Code changes
An earlier version of this PR added some new concepts but they will create more confusion than benefit. We should revisit the idea of how to swap out the ServiceManager from an extension in another PR.
- ~~
ServiceManager.getInstance(): ServiceManager.IManager | nullYou get either the currently existing service manager ornull.~~ - ~~
ServiceManager.setInstance(manager: ServiceManager.IManager): ServiceManager.IManagerYou can set the service manager instance (and trigger any already existing one todispose()~~ - ~~Since the
ServiceManager.IManagerinterface is an observable disposable, this allows theJupyterFrontEndto discover that its service manager has been disposed and then set it to a new service manager~~
User-facing changes
Backwards-incompatible changes
Thanks for making a pull request to jupyterlab!
To try out this branch on binder, follow this link:
Adds a static method
ServiceManager.getInstance()that returns aServiceManagerif one has already been instantiated. This allows an extension to directly import aServiceManagerinstance without threading it through each component that resides in the stack between where an extension was activated and where the component in question resides.
Does that mean we will expect extensions and plugins to use ServiceManager.getInstance() to access the service manager? And do we encourage this practice, or just provide an alternative?
Also wondering how this would work when a custom ServiceManager is provided to the JupyterLab application (or to a different JupyterFrontEnd). Would calling ServiceManager.getInstance() return the custom service manager instance?
@jtpio After thinking it through and discussing it with @blink1073, I think a better model is the following (I'll update the top-level comment as well):
ServiceManager.getInstance(): ServiceManager.IManager | nullYou get either the currently existing service manager ornull.ServiceManager.setInstance(manager: ServiceManager.IManager): ServiceManager.IManagerYou can set the service manager instance (and trigger any already existing one todispose()- Since the
ServiceManager.IManagerinterface is an observable disposable, this allows theJupyterFrontEndto discover that its service manager has been disposed and then set it to a new service manager:this._serviceManager = options.serviceManager || ServiceManager.getInstance() || ServiceManager.setInstance(new ServiceManager()); this._serviceManager.disposed.connect(() => { this._serviceManager = ServiceManager.getInstance() || ServiceManager.setInstance(new ServiceManager()); });
Does that mean we will expect extensions and plugins to use ServiceManager.getInstance() to access the service manager? And do we encourage this practice, or just provide an alternative?
This is just another alternative to try to simplify the structure of some components and not require them to thread down the JupyterFrontEnd#serviceManager but it isn't prescriptive about how somebody should get a service manager.
Also wondering how this would work when a custom ServiceManager is provided to the JupyterLab application (or to a different JupyterFrontEnd). Would calling ServiceManager.getInstance() return the custom service manager instance?
Yes, in this model it would return the custom one. The idea is that perhaps an extension might want to globally replace the service manager the application uses.
This PR adds support for the newly added
api/eventsendpoints injupyter_serverin@jupyterlab/services.
Should this require a bump to the jupyter_server dependency lower bound?
EDIT: ah this already seems to be the case on the latest commit :+1:
https://github.com/jupyterlab/jupyterlab/blob/4847b1b1df6e1e64a9411f0e78291876a137e6a1/pyproject.toml#L44
EDIT: ah this already seems to be the case on the latest commit +1
Since this is already the case the following TODO can probably be marked as completed?
- [ ] Remove capability probing for api/events in EventManager#_subscribe() after jupyter_server 2 becomes a hard requirement.
This version of the PR can be merged into the 3.6 branch, but a follow-on
PR should remove the capability probing block and should also remove the
Private.Stream implementation.
On Tue, 15 Nov 2022 at 15:16 Jeremy Tuloup @.***> wrote:
@.**** commented on this pull request.
In packages/services/src/event/index.ts https://github.com/jupyterlab/jupyterlab/pull/12667#discussion_r1022921252 :
- const response = await makeRequest(url, init, serverSettings);
- if (response.status !== 204) {
throw new ResponseError(response);- }
- }
- /**
- Subscribe to event bus emissions.
- */
- private async _subscribe(): Promise
{ - if (this.isDisposed) {
return;- }
- // TODO: (1) Make the test emission a request with an appropriate schema.
- // TODO: (2) Remove try block when jupyter_server 2 is a hard requirement.
Looks like this can already be done now since the current master has that hard requirement:
https://github.com/jupyterlab/jupyterlab/blob/4847b1b1df6e1e64a9411f0e78291876a137e6a1/pyproject.toml#L44
— Reply to this email directly, view it on GitHub https://github.com/jupyterlab/jupyterlab/pull/12667#pullrequestreview-1181061705, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABG6KIYVIOLCQGYADMAOZ3WIOSNVANCNFSM5YEOOLRQ . You are receiving this because you were assigned.Message ID: @.***>
ah ok, adding to the 3.6 milestone then if it was meant to be backported.
Although the 3.6 branch does not include the changes from https://github.com/jupyterlab/jupyterlab/pull/11537.
So it's not clear whether it can still be backward compatible because that will likely require changing the ServiceManager.IManager interface.
Although the 3.6 branch does not include the changes from https://github.com/jupyterlab/jupyterlab/pull/11537.
So it's not clear whether it can still be backward compatible because that will likely require changing the
ServiceManager.IManagerinterface.
The events manager can be added as optional in the interface in 3.6.x. so it stays backward compatible.
Based on the discussions above here are a couple of follow-ups to track in separate issues and PRs:
- [ ] In
EventManager#_subscribe(), make the test emission a request with a registered schema. This requires registering a schema with the back-end application's event bus. - [ ] Remove capability probing for
api/eventsinEventManager#_subscribe()afterjupyter_server2 becomes a hard requirement. - [ ] Write tests for
EventManagerin the@jupyterlab/servicestest suite. - [ ] Make
eventsoptional when backporting to 3.6.x
Maybe making EventManager optional should be in this PR since it's basically already got outstanding tasks to make it 4.0-ready.
@fcollonval Thank you for helping with the test failures!
I added a couple commits that switch to checking PageConfig for the server version instead of making a request (cc: @blink1073 @Zsailer) and I switched to using a Poll instance to manage backing off when socket connections fail so that we don't flood re-connection attempts.
@meeseeksdev please backport to 3.6.x
Oops, something went wrong applying the patch ... Please have a look at my logs.