Add async contents manager support #1328
Conversation
|
Thank you for making this pull request. Did you know? You can try it on Binder: Also, the version of Jupytext developed in this PR can be installed with (this requires |
|
@mwouts As we begin merging these two files, should we wrap all asynchronous functions (that might be synchronous in the synchronous contents manager) using Looking forward to your thoughts! |
Wow, thank you @Darshan808 , this is truly amazing!! Let me have a look! |
There was a problem hiding this comment.
Thank you @Darshan808 , I love what I see so far!
As we begin merging these two files, should we wrap all asynchronous functions (that might be synchronous in the synchronous contents manager) using
ensure_async(), similar to what was done in #1021?
Do you think it would be possible to do so? It would be great indeed if we had a more local change - easier to review, and just one file to maintain later on. But I must admit that it's precisely what I was not able to do two years ago so no obligation at all!
| try: | ||
| from jupytext.async_contentsmanager import build_jupytext_async_contents_manager_class | ||
| except ImportError as err: | ||
| build_jupytext_async_contents_manager = reraise(err) |
There was a problem hiding this comment.
Yes that's the most prudent approach, thank you! The import problem was years ago though, so maybe it's gone, but it's safer that I follow-up on that later on.
| if asyncio.iscoroutinefunction(base_class.get): | ||
| app.contents_manager_class = build_jupytext_async_contents_manager_class(base_class) | ||
| else: | ||
| app.contents_manager_class = build_jupytext_contents_manager_class(base_class) |
There was a problem hiding this comment.
Yes exactly what I had in mind! And we can delete the paragraph above.
|
Re the failing pre-commit hook on the CI you probably just need |
|
@mwouts |
Fixes #1316
This PR introduces support for an async contents manager. As suggested by @mwouts in #1316, a separate function,
build_jupytext_async_contents_manager, has been implemented in a new file,async_contentsmanager.py.To ensure full test coverage, the existing tests have been duplicated into
test_async_contentsmanager.py. Currently, all tests are passing! ✅