Skip to content
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

Implement a fps plugin for Voila #977

Closed
jtpio opened this issue Sep 24, 2021 · 5 comments
Closed

Implement a fps plugin for Voila #977

jtpio opened this issue Sep 24, 2021 · 5 comments
Labels
enhancement New feature or request
Milestone

Comments

@jtpio
Copy link
Member

jtpio commented Sep 24, 2021

Problem

In the Jupyter Server Meeting yesterday, we discussed the use case of having a fps plugin for Voila. This would be a good example for having a tihrd-party plugin living in a different repo.

fps and jupyverse are still experimental projects, but since jupyverse relies on entry_points to load the plugins, we could imagine having the fps-voila plugin as part of the voila package already.

This could be shipped as an early preview and released as part of the 0.3.x series.

It would also help refactor the voila logic so it can be used with both jupyter_server extensions and jupverse plugins.

Proposed Solution

We add a fps-voila plugin some time in the 0.3.0 release (additive change).

Additional context

@jtpio jtpio added the enhancement New feature or request label Sep 24, 2021
@jtpio jtpio added this to the 0.3.0 milestone Sep 24, 2021
@jtpio jtpio changed the title Implement a FPS plugin for Voila Implement a fps plugin for Voila Sep 24, 2021
@davidbrochart
Copy link
Member

I am implementing a fps-voila plugin, and I wanted to share some thoughts about the difficulties I am encountering:

  • Voila is tightly coupled with Tornado's API. In particular, for progressive rendering, Voila sends parts of the HTML. This seems hard to do with FastAPI.
  • Voila is a configurable application. It creates the Tornado handlers when it is initialized, meaning the endpoints themselves could depend on configuration. But FPS's plugin system requires that endpoints are known at import-time, which occurs before Voila is configured.
  • Voila is tightly coupled with nbclient for the notebook execution, which in turn is tightly coupled with jupyter_client's kernel managers and clients. Reusing fps-kernels requires wrapping a KernelServer so that it's accessible through jupyter_client's API.

@SylvainCorlay
Copy link
Member

Voila is tightly coupled with Tornado's API. In particular, for progressive rendering, Voila sends parts of the HTML. This seems hard to do with FastAPI.

Maybe @maartenbreddels has a clue on how other frameworks than tornado enable partial responses to HTTP requests.

Voila is tightly coupled with nbclient for the notebook execution, which in turn is tightly coupled with jupyter_client's kernel managers and clients. Reusing fps-kernels requires wrapping a KernelServer so that it's accessible through jupyter_client's API.

I am not sure this is really an issue since the notebook execution through the executor is independent from the fps-kernels thingy.

@trungleduc
Copy link
Member

trungleduc commented Sep 29, 2021

  • Voila is tightly coupled with Tornado's API. In particular, for progressive rendering, Voila sends parts of the HTML. This seems hard to do with FastAPI.

StreamingResponse from starlette might help? It's also available in fastapi (https://github.com/tiangolo/fastapi/blob/master/fastapi/responses.py#L9)

@davidbrochart
Copy link
Member

Indeed, thanks @trungleduc !

@davidbrochart
Copy link
Member

I am not sure this is really an issue since the notebook execution through the executor is independent from the fps-kernels thingy.

You are right, we can keep the jupyter_client kernel management in the backend, and use the kernel's connection information in KernelServer which talks to the frontend.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants