Skip to content

Commit

Permalink
Fix file id not available for jupyter-server
Browse files Browse the repository at this point in the history
Improve notebook cell server-side executor

Fix for testing drive

Allow to request a document from its document_id/room_id

Add documentation

Rename state room_id to document_id

Don't include custom logic for jupyter_server_nbmodel
  • Loading branch information
fcollonval committed May 20, 2024
1 parent 0cc728a commit a207d33
Show file tree
Hide file tree
Showing 13 changed files with 225 additions and 204 deletions.
1 change: 1 addition & 0 deletions .eslintignore
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
**/*.d.ts
**/test
**/ui-tests
**/labextension

docs
tests
Expand Down
1 change: 1 addition & 0 deletions .prettierignore
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
**/tsconfig.test.json
**/*.d.ts
**/test
**/labextension

docs
tests
Expand Down
2 changes: 1 addition & 1 deletion packages/collaboration-extension/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@
},
"devDependencies": {
"@jupyterlab/builder": "^4.0.5",
"@types/react": "~18.0.26",
"@types/react": "~18.3.1",
"npm-run-all": "^4.1.5",
"rimraf": "^4.1.2",
"typescript": "~5.0.4"
Expand Down
2 changes: 1 addition & 1 deletion packages/collaboration/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@
"yjs": "^13.5.40"
},
"devDependencies": {
"@types/react": "^18.0.27",
"@types/react": "~18.3.1",
"rimraf": "^4.1.2",
"typescript": "~5.0.4"
},
Expand Down
4 changes: 2 additions & 2 deletions packages/docprovider-extension/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,8 @@
"yjs": "^13.5.40"
},
"devDependencies": {
"@jupyterlab/builder": "^4.0.5",
"@types/react": "~18.0.26",
"@jupyterlab/builder": "^4.0.0",
"@types/react": "~18.3.1",
"npm-run-all": "^4.1.5",
"rimraf": "^4.1.2",
"typescript": "~5.0.4"
Expand Down
55 changes: 5 additions & 50 deletions packages/docprovider-extension/src/executor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,12 @@
* @module docprovider-extension
*/

import { NotebookCellServerExecutor } from '@jupyter/docprovider';
import {
JupyterFrontEnd,
JupyterFrontEndPlugin
} from '@jupyterlab/application';
import { PageConfig, URLExt } from '@jupyterlab/coreutils';
import { ServerConnection } from '@jupyterlab/services';

import { type MarkdownCell } from '@jupyterlab/cells';
import { PageConfig } from '@jupyterlab/coreutils';
import { INotebookCellExecutor, runCell } from '@jupyterlab/notebook';

export const notebookCellExecutor: JupyterFrontEndPlugin<INotebookCellExecutor> =
Expand All @@ -24,53 +22,10 @@ export const notebookCellExecutor: JupyterFrontEndPlugin<INotebookCellExecutor>
provides: INotebookCellExecutor,
activate: (app: JupyterFrontEnd): INotebookCellExecutor => {
if (PageConfig.getOption('serverSideExecution') === 'true') {
return Object.freeze({ runCell: runCellServerSide });
return new NotebookCellServerExecutor({
serverSettings: app.serviceManager.serverSettings
});
}
return Object.freeze({ runCell });
}
};

async function runCellServerSide({
cell,
notebook,
notebookConfig,
onCellExecuted,
onCellExecutionScheduled,
sessionContext,
sessionDialogs,
translator
}: INotebookCellExecutor.IRunCellOptions): Promise<boolean> {
switch (cell.model.type) {
case 'markdown':
(cell as MarkdownCell).rendered = true;
cell.inputHidden = false;
onCellExecuted({ cell, success: true });
break;
case 'code': {
const kernelId = sessionContext?.session?.kernel?.id;
const settings = ServerConnection.makeSettings();
const apiURL = URLExt.join(
settings.baseUrl,
`api/kernels/${kernelId}/execute`
);
const cellId = cell.model.sharedModel.getId();
const documentId = `json:notebook:${notebook.sharedModel.getState(
'file_id'
)}`;
const body = `{"cell_id":"${cellId}","document_id":"${documentId}"}`;
const init = {
method: 'POST',
body
};
try {
await ServerConnection.makeRequest(apiURL, init, settings);
} catch (error: any) {
throw new ServerConnection.NetworkError(error);
}
break;
}
default:
break;
}
return Promise.resolve(true);
}
11 changes: 8 additions & 3 deletions packages/docprovider/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -42,17 +42,22 @@
},
"dependencies": {
"@jupyter/ydoc": "^2.0.0",
"@jupyterlab/coreutils": "^6.0.5",
"@jupyterlab/services": "^7.0.5",
"@jupyterlab/apputils": "^4.2.0",
"@jupyterlab/cells": "^4.2.0",
"@jupyterlab/coreutils": "^6.2.0",
"@jupyterlab/notebook": "^4.2.0",
"@jupyterlab/services": "^7.2.0",
"@jupyterlab/translation": "^4.2.0",
"@lumino/coreutils": "^2.1.0",
"@lumino/disposable": "^2.1.0",
"@lumino/signaling": "^2.1.0",
"@lumino/widgets": "^2.2.0",
"y-protocols": "^1.0.5",
"y-websocket": "^1.3.15",
"yjs": "^13.5.40"
},
"devDependencies": {
"@jupyterlab/testing": "^4.0.5",
"@jupyterlab/testing": "^4.0.0",
"@types/jest": "^29.2.0",
"jest": "^29.5.0",
"rimraf": "^4.1.2",
Expand Down
2 changes: 2 additions & 0 deletions packages/docprovider/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
*/

export * from './awareness';
export * from './notebookCellExecutor';
export * from './requests';
export * from './ydrive';
export * from './yprovider';
export * from './tokens';
138 changes: 138 additions & 0 deletions packages/docprovider/src/notebookCellExecutor.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,138 @@
/* -----------------------------------------------------------------------------
| Copyright (c) Jupyter Development Team.
| Distributed under the terms of the Modified BSD License.
|----------------------------------------------------------------------------*/

import { Dialog, showDialog } from '@jupyterlab/apputils';
import { type ICodeCellModel, type MarkdownCell } from '@jupyterlab/cells';
import { URLExt } from '@jupyterlab/coreutils';
import { INotebookCellExecutor } from '@jupyterlab/notebook';
import { ServerConnection } from '@jupyterlab/services';
import { nullTranslator } from '@jupyterlab/translation';

/**
* Notebook cell executor posting a request to the server for execution.
*/
export class NotebookCellServerExecutor implements INotebookCellExecutor {
private _serverSettings: ServerConnection.ISettings;

/**
* Constructor
*
* @param options Constructor options; the contents manager, the collaborative drive and optionally the server settings.
*/
constructor(options: { serverSettings?: ServerConnection.ISettings }) {
this._serverSettings =
options.serverSettings ?? ServerConnection.makeSettings();
}

/**
* Execute a given cell of the notebook.
*
* @param options Execution options
* @returns Execution success status
*/
async runCell({
cell,
notebook,
notebookConfig,
onCellExecuted,
onCellExecutionScheduled,
sessionContext,
sessionDialogs,
translator
}: INotebookCellExecutor.IRunCellOptions): Promise<boolean> {
translator = translator ?? nullTranslator;
const trans = translator.load('jupyterlab');

switch (cell.model.type) {
case 'markdown':
(cell as MarkdownCell).rendered = true;
cell.inputHidden = false;
onCellExecuted({ cell, success: true });
break;
case 'code':
if (sessionContext) {
if (sessionContext.isTerminating) {
await showDialog({
title: trans.__('Kernel Terminating'),
body: trans.__(
'The kernel for %1 appears to be terminating. You can not run any cell for now.',
sessionContext.session?.path
),
buttons: [Dialog.okButton()]
});
break;
}
if (sessionContext.pendingInput) {
await showDialog({
title: trans.__('Cell not executed due to pending input'),
body: trans.__(
'The cell has not been executed to avoid kernel deadlock as there is another pending input! Submit your pending input and try again.'
),
buttons: [Dialog.okButton()]
});
return false;
}
if (sessionContext.hasNoKernel) {
const shouldSelect = await sessionContext.startKernel();
if (shouldSelect && sessionDialogs) {
await sessionDialogs.selectKernel(sessionContext);
}
}

if (sessionContext.hasNoKernel) {
cell.model.sharedModel.transact(() => {
(cell.model as ICodeCellModel).clearExecution();
});
return true;
}

const kernelId = sessionContext?.session?.kernel?.id;
const apiURL = URLExt.join(
this._serverSettings.baseUrl,
`api/kernels/${kernelId}/execute`
);
const cellId = cell.model.sharedModel.getId();
const documentId = notebook.sharedModel.getState('document_id');

const init = {
method: 'POST',
body: JSON.stringify({ cell_id: cellId, document_id: documentId })
};
onCellExecutionScheduled({ cell });
let success = false;
try {
// FIXME quid of deletedCells and timing record
const response = await ServerConnection.makeRequest(
apiURL,
init,
this._serverSettings
);
success = response.ok;
} catch (error: unknown) {
onCellExecuted({
cell,
success: false
});
if (cell.isDisposed) {
return false;
} else {
throw error;
}
}

onCellExecuted({ cell, success });

return true;
}
cell.model.sharedModel.transact(() => {
(cell.model as ICodeCellModel).clearExecution();
}, false);
break;
default:
break;
}
return Promise.resolve(true);
}
}
7 changes: 6 additions & 1 deletion packages/docprovider/src/yprovider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -133,8 +133,13 @@ export class WebSocketProvider implements IDocumentProvider {

private _onSync = (isSynced: boolean) => {
if (isSynced) {
if (this._yWebsocketProvider) {
this._yWebsocketProvider.off('sync', this._onSync);

const state = this._sharedModel.ydoc.getMap('state');
state.set('document_id', this._yWebsocketProvider.roomname);
}
this._ready.resolve();
this._yWebsocketProvider?.off('sync', this._onSync);
}
};

Expand Down
30 changes: 22 additions & 8 deletions projects/jupyter-server-ydoc/jupyter_server_ydoc/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -136,21 +136,35 @@ def initialize_handlers(self):
async def get_document(
self: YDocExtension,
*,
path: str,
content_type: Literal["notebook", "file"],
file_format: Literal["json", "text"],
path: str | None = None,
content_type: str | None = None,
file_format: Literal["json", "text"] | None = None,
room_id: str | None = None,
copy: bool = True,
) -> YBaseDoc | None:
"""Get a view of the shared model for the matching document.
You need to provide either a ``room_id`` or the ``path``,
the ``content_type`` and the ``file_format``.
If `copy=True`, the returned shared model is a fork, meaning that any changes
made to it will not be propagated to the shared model used by the application.
"""
file_id_manager = self.serverapp.web_app.settings["file_id_manager"]
file_id = file_id_manager.index(path)
error_msg = "You need to provide either a ``room_id`` or the ``path``, the ``content_type`` and the ``file_format``."
if room_id is None:
if path is None or content_type is None or file_format is None:
raise ValueError(error_msg)

file_id_manager = self.serverapp.web_app.settings["file_id_manager"]
file_id = file_id_manager.index(path)

encoded_path = encode_file_path(file_format, content_type, file_id)
room_id = room_id_from_encoded_path(encoded_path)

encoded_path = encode_file_path(file_format, content_type, file_id)
room_id = room_id_from_encoded_path(encoded_path)
elif path is not None or content_type is not None or file_format is not None:
raise ValueError(error_msg)
else:
room_id = room_id

try:
room = await self.ywebsocket_server.get_room(room_id)
Expand All @@ -164,7 +178,7 @@ async def get_document(
fork_ydoc = Doc()
fork_ydoc.apply_update(update)

return YDOCS.get(content_type, YDOCS["file"])(fork_ydoc)
return YDOCS.get(room.file_type, YDOCS["file"])(fork_ydoc)
else:
return room._document

Expand Down
10 changes: 10 additions & 0 deletions projects/jupyter-server-ydoc/jupyter_server_ydoc/rooms.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,16 @@ def __init__(
self._document.observe(self._on_document_change)
self._file.observe(self.room_id, self._on_outofband_change)

@property
def file_format(self) -> str:
"""Document file format."""
return self._file_format

@property
def file_type(self) -> str:
"""Document file type."""
return self._file_type

@property
def room_id(self) -> str:
"""
Expand Down
Loading

0 comments on commit a207d33

Please sign in to comment.