Skip to content

Diagram fix: Kernel doesn't update Y doc, Server does #41

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

Merged
merged 3 commits into from
Jun 25, 2025

Conversation

cben
Copy link
Contributor

@cben cben commented May 28, 2025

Based on my understanding of code, please check

Before | After

  • Kernel doesn't touch Y doc, it's the Server that updates it
  • also moved Shared document left between Frontend and Server
  • using actor stick figure shapes for actual separate processes
    • "Shared Document" is abstraction, actually exists as separate Y models inside Frontend(s) and inside Server. But I think it's fine for this diagram.
    • ExecutionStack is an object inside Server. Mermaid can group participants but that might look too messy?
      • WDYT re-wording to e.g. "put() request into queue", "get() task result" to imply these are local function calls, not network messages?
  • After execute_reply, I wasn't sure if diagram should elaborate on Server updating the Y model with 'idle' state & count? It does, but it also returns them over HTTP as already shown in diagram; not sure which drives notebook changing [*] -> [15]...

- also moved Shared document left between Frontend and Server
- using `actor` stick figure shapes for actual separate processes
Copy link

Binder 👈 Launch a Binder on branch cben/jupyter-server-nbmodel/diagram

@krassowski
Copy link
Collaborator

not sure which drives notebook changing [*] -> [15].

It is the y model which does that in recent versions (otherwise if you closed the browser when the execution completed, the state would not be updated).

@cben
Copy link
Contributor Author

cben commented Jun 23, 2025

Updated, PTAL. Before | After links to the branch, so reflects the new commits.

Copy link
Member

@echarles echarles left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM Thx @cben

@echarles echarles added the documentation Improvements or additions to documentation label Jun 25, 2025
@echarles echarles merged commit 03e22c0 into datalayer:main Jun 25, 2025
4 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants