Skip to content

Fix: sync error #148

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.

Sign up for GitHub

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

Jump to bottom
Merged
echarles merged 2 commits into from
Oct 27, 2025
Merged

Fix: sync error #148

echarles merged 2 commits into from
Oct 27, 2025

Conversation

@ChengJiale150
Copy link
Contributor

@ChengJiale150 ChengJiale150 commented Oct 27, 2025 •
edited
Loading

fix the issue #146

add thread lock and transaction to fix sync bug in insert_cell and overwrite_cell refer to the implement in jupyter-nbmodel-client.

It work well in my local test, and no sync problem occur.

@ChengJiale150
Copy link
Contributor Author

ChengJiale150 commented Oct 27, 2025

In Fact, this is as same as the code in jupyter-nbmodel-client

@echarles
Copy link
Member

echarles commented Oct 27, 2025

In Fact, this is as same as the code in jupyter-nbmodel-client

Instead of duplicating the code,could we just call jupyter-nbmodel-client or just not having a special case for JUPYTER_SERVER and do the same as for the MCP_SERVER case?

@ChengJiale150
Copy link
Contributor Author

ChengJiale150 commented Oct 27, 2025 •
edited
Loading

It will be imporved in future depends on research of the difference between serverapp and websocket.

@ChengJiale150 ChengJiale150 marked this pull request as draft October 27, 2025 12:11
@echarles
Copy link
Member

echarles commented Oct 27, 2025

we should just make it simpler, and use the MCP_SERVER case when needed for JUPYTER_SERVER

@ChengJiale150 ChengJiale150 marked this pull request as ready for review October 27, 2025 13:43
@ChengJiale150
Copy link
Contributor Author

ChengJiale150 commented Oct 27, 2025 •
edited
Loading

use same NotebookModel in jupyter-nbmode-client to simplify the api in JUPYTER_SERVER Mode.

echarles
echarles approved these changes Oct 27, 2025
View reviewed changes
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.

Tested and works for me. Thx @ChengJiale150

@echarles echarles merged commit 2358010 into datalayer:main Oct 27, 2025
15 checks passed
@echarles
Copy link
Member

echarles commented Oct 27, 2025

0.18.1 released with this fix https://github.com/datalayer/jupyter-mcp-server/releases/tag/v0.18.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@echarles echarles echarles approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ChengJiale150 @echarles