-
Notifications
You must be signed in to change notification settings - Fork 6
Handle in-band/out-of-band file moves & deletions #103
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
Conversation
3coins
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dlqqq
This looks great! 🚀
Don't have much suggestions on the code. I was able to test most of the scenarios, which worked ok, other than the dirty flag and reported editor issues. One new issue I noticed is that occasionally the notebook content totally vanishes after an out-of-band change. I last saw this when I renamed (in-band) a file, and then made an out-of-band edit. Here was the exception from logs.
[W 2025-06-06 08:39:24.089 ServerDocsApp] File was moved to 'duplicate-test-rename.ipynb'. Room ID: 'json:notebook:d4299f70-255c-4735-93d5-667689dd012e', Last known path: 'duplicate-test.ipynb'.
[W 2025-06-06 08:40:18.897 ServerDocsApp] Out-of-band file change detected. Room ID: 'json:notebook:d4299f70-255c-4735-93d5-667689dd012e', Last detected change: '2025-06-06 15:39:18.555275+00:00', Most recent change: '2025-06-06 15:40:18.608199+00:00'.
[I 2025-06-06 08:40:18.898 ServerDocsApp] Loading content for room ID 'json:notebook:d4299f70-255c-4735-93d5-667689dd012e'.
[I 2025-06-06 08:40:18.900 ServerDocsApp] Stopped `self._watch_file()` background task for YRoom 'json:notebook:d4299f70-255c-4735-93d5-667689dd012e'.
[W 2025-06-06 08:40:18.907 ServerApp] Notebook duplicate-test-rename.ipynb is not trusted
[I 2025-06-06 08:40:18.908 ServerDocsApp] Loaded content for room ID 'json:notebook:d4299f70-255c-4735-93d5-667689dd012e'.
[I 2025-06-06 08:40:18.914 ServerDocsApp] Initializing room 'json:notebook:6fcc4fe2-121f-413e-b77a-b48328d8a1e5'.
[I 2025-06-06 08:40:18.914 ServerDocsApp] Loading content for room ID 'json:notebook:6fcc4fe2-121f-413e-b77a-b48328d8a1e5'.
[I 2025-06-06 08:40:18.915 ServerDocsApp] Room 'json:notebook:6fcc4fe2-121f-413e-b77a-b48328d8a1e5' initialized.
Task exception was never retrieved
future: <Task finished name='Task-19315' coro=<YRoomFileAPI._load_ydoc_content() done, defined at /Users/pijain/projects/jupyter-server-documents/jupyter_server_documents/rooms/yroom_file_api.py:163> exception=HTTPError()>
Traceback (most recent call last):
File "/Users/pijain/projects/jupyter-server-documents/jupyter_server_documents/rooms/yroom_file_api.py", line 171, in _load_ydoc_content
file_data = await ensure_async(self._contents_manager.get(
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
...<3 lines>...
))
^^
File "/Users/pijain/miniforge3/envs/serverdocs/lib/python3.13/site-packages/jupyter_core/utils/__init__.py", line 197, in ensure_async
result = await obj
^^^^^^^^^
File "/Users/pijain/miniforge3/envs/serverdocs/lib/python3.13/site-packages/jupyter_server/services/contents/filemanager.py", line 910, in get
raise web.HTTPError(404, "No such file or directory: %s" % path)
tornado.web.HTTPError: HTTP 404: Not Found (No such file or directory: duplicate-test.ipynb)|
@3coins Thanks for the review & for sharing the edge case you encountered. I'll try this out myself and see if I'm able to reproduce the issue you reported. EDIT: I was able to reproduce the issue. It looks like the frontend is requesting a different room ID than the one from before the out-of-band change, even though the file ID should be the same after a file was moved in-band. |
|
@3coins The bug has been fixed by only requesting the file ID once. That way, the room ID stays the same if the file was moved in-band, preventing the error you reported. I found a new bug with This requires an upstream fix and seems to happen when mixing a bunch of out-of-band deletes & in-band moves, which we are using to test these branches. If you run into this issue, you can still test the fix by moving the file in-band to a unique filename. |
3coins
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Thanks.
Description
This PR implements UI handling for the following edge cases:
In-band move (file moved via
ContentsManager)ArbitraryFileIdManagerprovided byjupyter_server_fileid. The backend has been modified to just log when this happens. No notification appears for the user, as the user does not need to take any action.In-band deletion (file deleted via
ContentsManager): if the document tab is open, closes the tab & notifies the user.Out-of-band move / deletion (file was either moved or deleted without the
ContentsManager): if the document tab is open, closes the tab & notifies the user.LocalFileIdManagerprovided byjupyter_server_fileid. This is not enabled by default because it only works on local filesystems.This PR also improves the in-band & out-of-band notifications:
These notifications will show only if the document is open (i.e. the document has a tab open in the user's browser). Because this branch now handles all in-band & out-of-band operations gracefully, the user doesn't need to be notified unless they have the document open.
For out-of-band changes, the notification only appears when the user has the document open AND the document is visible, i.e. the user can see the document directly.
These notifications now include the path of the file that was changed/moved/deleted.
These notifications now automatically dismiss themselves in 10 seconds.
Demos
In-band move
Screen.Recording.2025-06-05.at.3.42.45.PM.mov
In-band deletion
This case requires a second browser to test, since if a client opens a document and deletes it from their UI, the UI state updates automatically by default. The previously unhandled case is when another client deletes the file in-band.
Screen.Recording.2025-06-05.at.3.45.06.PM.mov
Out-of-band move/deletion
Screen.Recording.2025-06-05.at.3.46.40.PM.mov
Technical details
Handling for in-band deletions & out-of-band moves/deletions is implemented similarly to #83. Special close codes indicate when either scenario occurs:
The only difference is that for 4001/4002, we do not reconnect and close the document tab instead, as the file is no longer available for editing.
To find, query, and close the current document tab, the
WebsocketProviderhas been modified to take an additionalapp: JupyterFrontEndargument in its constructor. By iterating throughapp.shell.widgets(), it can find the document tab that hosts its shared model.