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

Support AsyncContentsManager? #1316

Open
krassowski opened this issue Feb 13, 2025 · 1 comment · May be fixed by #1328
Open

Support AsyncContentsManager? #1316

krassowski opened this issue Feb 13, 2025 · 1 comment · May be fixed by #1328
Milestone

Comments

@krassowski
Copy link

It would be great if jupytext supported usage with AsyncContentsManager. Currently such contents manager are explicitly ignored:

# If possible, we derive a Jupytext CM from the current CM
base_class = app.contents_manager_class
if asyncio.iscoroutinefunction(base_class.get):
app.log.warning(
f"[Jupytext Server Extension] Async contents managers like {base_class.__name__} "
"are not supported at the moment "
"(https://github.com/mwouts/jupytext/issues/1020). "
"We will derive a contents manager from LargeFileManager instead."
)
from jupyter_server.services.contents.largefilemanager import ( # noqa
LargeFileManager,
)
base_class = LargeFileManager

It looks like changes would need to be made to

def build_jupytext_contents_manager_class(base_contents_manager_class):

to support deriving from AsyncContentsManager.

Are there specific reasons for not allowing it, or is it just that it needs to be implemented?

@mwouts
Copy link
Owner

mwouts commented Feb 14, 2025

Hey @krassowski , thank you for reaching out! Yes I would love to support this but so far I have not yet found nor the time nor the proper way to do this.

There is a very old PR that relates to this: #1021, but the change had a huge impact on tests etc so I could not get that in safely.

Maybe it would be better to duplicate the build_jupytext_contents_manager function to a new file that would implement the async support, and duplicate the tests as well to focus on the async support in new test files without impacting too much the existing (non-async) ones?

@mwouts mwouts added this to the 1.17.0 milestone Feb 16, 2025
@Darshan808 Darshan808 linked a pull request Feb 20, 2025 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants