Clean up handling of synchronous managers #1044
Conversation
Codecov ReportBase: 73.10% // Head: 73.08% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #1044 +/- ##
==========================================
- Coverage 73.10% 73.08% -0.03%
==========================================
Files 64 64
Lines 8274 8269 -5
Branches 1643 1642 -1
==========================================
- Hits 6049 6043 -6
- Misses 1816 1820 +4
+ Partials 409 406 -3
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
| @@ -1435,38 +1433,13 @@ def template_file_path(self): | |||
| help="""If True, display controls to shut down the Jupyter server, such as menu items or buttons.""", | |||
| ) | |||
|
|
|||
| # REMOVE in VERSION 2.0 | |||
There was a problem hiding this comment.
@Zsailer I believe removing this is what is causing the nbclassic downstream test to fail. Do we want to keep this in for another major release?
There was a problem hiding this comment.
Ah, right.
Yes. We can wait another release. I don't see any major urgency to removing this now, we just don't want to keep this around forever.
There was a problem hiding this comment.
Okay, punted to 3.0
There was a problem hiding this comment.
No, that was not it. Now I'm confused.
There was a problem hiding this comment.
Okay, so the actual issue occurs when we change the default value from LargeFileManager -> AsyncLargeFileManager, which was the main idea of this change. 😅
There was a problem hiding this comment.
Ah, nbclassic was assuming synchronous contents manager:
tests/test_notebookapp.py::test_tree_handler
/home/runner/test_venv/downstream_test/nbclassic/nbclassic/tree/handlers.py:53: RuntimeWarning: coroutine 'AsyncFileContentsManager.dir_exists' was never awaited
if cm.dir_exists(path=path):
Enable tracemalloc to get traceback where the object was allocated.
See https://docs.pytest.org/en/stable/how-to/capture-warnings.html#resource-warnings for more info.
tests/test_notebookapp.py::test_tree_handler
/home/runner/test_venv/downstream_test/nbclassic/nbclassic/tree/handlers.py:54: RuntimeWarning: coroutine 'AsyncFileContentsManager.is_hidden' was never awaited
We need to update it regardless.
|
All green! |
Make the async contents manager the default, and add deprecation warning when using synchronous contents or kernel managers.