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

Should code be executed asynchronously in RTC mode? #7

Closed
Wh1isper opened this issue May 8, 2024 · 3 comments · Fixed by #9
Closed

Should code be executed asynchronously in RTC mode? #7

Wh1isper opened this issue May 8, 2024 · 3 comments · Fixed by #9

Comments

@Wh1isper
Copy link

Wh1isper commented May 8, 2024

Problem

reply = await ensure_async(
client.execute_interactive(
snippet,
output_hook=self._output_hook,
stdin_hook=self._stdin_hook if client.allow_stdin else None,
)
)

I've found that current implementations where cell takes longer to execute can lead to http request timeouts (especially behind some reverse proxies), and it's difficult for the front-end to determine if the request to execute the code was successfully sent.

Proposed Solution

  1. We can simply support asynchronous mode: do execute and safe in an asyncio.Task.
  2. Maybe we need an optimistic lock to prevent racing when concurrent write to the ynotebook.

Additional context

@fcollonval
Copy link
Member

Hey @Wh1isper thanks for testing, actually we cannot switch to asynchronous mode fully to be able to support the input case.

I plan to implement the following logic that will cover that case:

  1. Return 202 with Location to get the execution result when receiving the request
  2. On the result endpoint, return early if an input is needed otherwise return the results

@Wh1isper
Copy link
Author

Return 202 with Location to get the execution result when receiving the request

It sounds like we can only perform one code execution operation at a time? Or we will relay on kernel's zmq queue?

@fcollonval
Copy link
Member

@Wh1isper I opened #9 which implements my proposal (it requires the latest version of jupyterlab/jupyter-collaboration#307), if you want to have a look.

The solution I have in mind for stdin does not work. Currently the server is locked on an input request. If you have any idea to work around that. I'll be happy to hear them.

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

Successfully merging a pull request may close this issue.

2 participants