-
Notifications
You must be signed in to change notification settings - Fork 316
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
Upgrade anyio to v3 #492
Upgrade anyio to v3 #492
Conversation
Excellent, thank you @mwakaba2 and @agronholm! I'm going to place this into DRAFT for now until Alex's anyio PR is merged and released. |
ipython_genutils is complicated to package on some linux distro because of nose, and in general not useful as it mostly offered python 2/3 compat layer. Goal always has been to remove any usage of ipython_genutils
This uses an autoreformatter to reformat and fix some of the common mistakes in numpydoc. Some of this is purely visual, but other like space before colon in parameters actually have semantic values. In the case of space before colon this is to make sure that nupmydoc properly distinguish the parameter name from its type.
This change also prevents permission-related exceptions from logging the file. Co-authored-by: Shane Canon <[email protected]> Co-authored-by: Thomas Kluyver <[email protected]>
280c3da
to
409b5b2
Compare
Codecov Report
@@ Coverage Diff @@
## master #492 +/- ##
==========================================
- Coverage 77.74% 77.71% -0.04%
==========================================
Files 106 106
Lines 9276 9288 +12
Branches 1001 1001
==========================================
+ Hits 7212 7218 +6
- Misses 1707 1711 +4
- Partials 357 359 +2
Continue to review full report at Codecov.
|
Good to see the quick turnaround on the anyio patch, although it looks like 3.6 still has some heartburn. Choices...
The PR for #490 will probably need option 1. |
FWIW, according to https://pypistats.org/packages/jupyter-server, Python 3.6 downloads account for 13%~36% on PyPI for the past 30 days. |
Thanks for the metric @yan12125 - that's good to know, although I suspect the percentage overriding the default contents manager to the async version is nearly zero. Nevertheless, I think the first option would be the best step forward (barring a more graceful solution) as it can also be used to accomplish #490. |
Yeah I think the first option sounds good to unblock #490. I'll also look into the issue with python 3.6 and check with anyio if it's related to their new update. |
If the problem with py3.6 is caused by AnyIO, I would like to know. |
@kevin-bates I'd appreciate your review on this PR. @agronholm I created a new issue for the py3.6 error. #505 It looks like it's an anyio issue. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These changes look good and allow us to proceed with #490. Thank you!
thank you @kevin-bates ! I don't have permission to merge the pull request, so if it all looks good, please feel free to merge it. |
Thanks Mariko - I was giving others a chance to comment, but these seem straightforward enough - so I'll go ahead. |
@mwakaba2, I've upgraded your permissions to "write" 😎 . My apologies for not doing this sooner! |
sweet! thanks so much. I look forward to contributing more! 😎 |
Upgrade anyio to v3
error when running unittest: /opt/hostedtoolcache/Python/3.8.18/x64/lib/python3.8/site-packages/_pytest/assertion/rewrite.py:178: in exec_module exec(co, module.__dict__) enterprise_gateway/tests/test_gatewayapp.py:11: in <module> from enterprise_gateway.enterprisegatewayapp import EnterpriseGatewayApp enterprise_gateway/enterprisegatewayapp.py:19: in <module> from jupyter_server.serverapp import random_ports /opt/hostedtoolcache/Python/3.8.18/x64/lib/python3.8/site-packages/jupyter_server/serverapp.py:77: in <module> from .services.contents.filemanager import AsyncFileContentsManager, FileContentsManager /opt/hostedtoolcache/Python/3.8.18/x64/lib/python3.8/site-packages/jupyter_server/services/contents/filemanager.py:15: in <module> from anyio import run_sync_in_worker_thread E ImportError: cannot import name 'run_sync_in_worker_thread' from 'anyio' (/opt/hostedtoolcache/Python/3.8.18/x64/lib/python3.8/site-packages/anyio/__init__.py) ref: jupyter-server/jupyter_server@678878f jupyter-server/jupyter_server#492
This fixes #487 (Anyio version 3 breaks AsyncContentsManager).
Changes
run_sync_in_worker_thread
withto_thread.run_sync
.jp
How to test
$ pip install -e ".[test]" $ pip install git+https://github.com/agronholm/anyio.git@fix-264 $ python -m pytest -v jupyter_server/tests/services/contents/
This PR depends on Anyio's Refactored asyncio thread pooling .
Todo