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

Register new file types #884

Closed
davidbrochart opened this issue Jun 20, 2022 · 8 comments
Closed

Register new file types #884

davidbrochart opened this issue Jun 20, 2022 · 8 comments

Comments

@davidbrochart
Copy link
Contributor

davidbrochart commented Jun 20, 2022

Problem

Currently, jupyter-server has the notion of file type, which is hard-coded to either "directory", "file" or "notebook". Any other type would be treated as invalid.
In jupyter-ydoc we have introduced the notion of Y document type. Current built-in types are YFile and YNotebook, which map to a "file" and a "notebook", respectively. This mapping is extensible using entry points.
So one can create new Y document types, but not jupyter-server document types. Say I have a new file type that I want to open with JupyterLab and want it to be collaborative, I can create a Y document for it but I won't have any other choice than have it seen as a jupyter-server "file".

Proposed Solution

I mostly have questions:

  • Should we introduce an extension mechanism in jupyter-server, that would allow to register a new file type, similarly to what we've done in jupyter-ydoc?
  • Should we only have "directory" and "file" in jupyter-server, and get rid of "notebook"? I know we validate notebook formats, but should it be the responsibility of the server?

Additional context

@trungleduc's PR.

@SylvainCorlay
Copy link
Contributor

I think that for our use case, it should always be file - and the special casing of the notebook type should probably go away.

@davidbrochart
Copy link
Contributor Author

I started the NBectomy and could remove quite a lot of code while keeping JupyterLab functional.

@kevin-bates
Copy link
Member

If we were to allow for the registration of Type Handlers as an attribute of the current ContentsManager would this provide a way forward? (Btw, the handler portion of this name should have no implication to a typical REST API handler.)

Practically speaking, the job of the ContentsManager is to persist and retrieve content. The FileContentsManager deals with content in the filesystem, SQLContentsManager deals with content in a SQL DB, etc. In all cases, notebooks are merely a file with two exceptions: 1. they are more rigidly validated, 2. they are checked relative to "trust" (kinda hand waving on that statement, but you get the idea). However, their persistence and retrieval are identical to "file".

If we let Type Handlers deal with those aspects of the "content" where the file type in the model identifies which TypeHandler to perform that action with, then perhaps we have a way forward. OOTB, Server would provide a NotebookTypeHandler but applications like RTC that perform "validation" and "trust" could unset or ignore NotebookTypeHandler or register their own instead. (I haven't really thought through how this machinery would look, so, more hand-waving.)

Types that are not "directory" or "file" and do not have a registered TypeHandler would be treated as "file". (In fact, you could argue that "directory" is really a special kind of "file".) We could also support the ability to register type handlers for "file" and "directory", if we wanted (and, again, providing defaults OOTB).

I'm not sure if this has any synergy to what has been done in RTC and I apologize for not following that more closely.

@davidbrochart
Copy link
Contributor Author

Thanks Kevin, I think this is exactly what we need if we want to register new file type handlers.

@vidartf
Copy link
Member

vidartf commented Jul 29, 2022

Does the merge of #895 close this, or are there still remaining tasks?

@davidbrochart
Copy link
Contributor Author

I'll close it for now, we can re-open if we need to.

@a3626a
Copy link
Contributor

a3626a commented Oct 6, 2022

I have found that custom file type is not allowed to be saved now. So we have remaining tasks and this issue should be re-opened.

PUT /api/contents/path/Name.ipynb does not change custom file type to 'file', while GET and POST do.

@web.authenticated
@authorized
async def put(self, path=""):
"""Saves the file in the location specified by name and path.
PUT is very similar to POST, but the requester specifies the name,
whereas with POST, the server picks the name.
PUT /api/contents/path/Name.ipynb
Save notebook at ``path/Name.ipynb``. Notebook structure is specified
in `content` key of JSON request body. If content is not specified,
create a new empty notebook.
"""
model = self.get_json_body()
cm = self.contents_manager
if model:
if model.get("copy_from"):
raise web.HTTPError(400, "Cannot copy with PUT, only POST")
if (
(model.get("path") and await ensure_async(cm.is_hidden(model.get("path"))))
or await ensure_async(cm.is_hidden(path))
) and not cm.allow_hidden:
raise web.HTTPError(400, f"Cannot create file or directory {path!r}")
exists = await ensure_async(self.contents_manager.file_exists(path))
if exists:
await self._save(model, path)
else:
await self._upload(model, path)
else:
await self._new_untitled(path)

This raises an error when I save my custom document.
image

@davidbrochart
Copy link
Contributor Author

Closing again, as #1013 was merged.

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

No branches or pull requests

5 participants