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

Drop dependency on async_generator #2212

Closed
wants to merge 10 commits into from
Closed

Conversation

agronholm
Copy link
Contributor

This removes all uses of async_generator, replacing it with contextlib2 in the cases where aclosing() is needed, and then only on Python < 3.10.

This depends on #2210.

The only thing left missing is aclosing() on Python < 3.10 which is now provided by contextlib2.
@codecov
Copy link

codecov bot commented Jan 16, 2022

Codecov Report

Merging #2212 (9dc1251) into master (39d01b2) will decrease coverage by 0.19%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #2212      +/-   ##
==========================================
- Coverage   99.57%   99.38%   -0.20%     
==========================================
  Files         114      114              
  Lines       14731    14724       -7     
  Branches     2334     2335       +1     
==========================================
- Hits        14669    14634      -35     
- Misses         41       70      +29     
+ Partials       21       20       -1     
Impacted Files Coverage Δ
trio/_core/_ki.py 95.77% <100.00%> (-4.23%) ⬇️
trio/_core/tests/test_asyncgen.py 99.06% <100.00%> (+<0.01%) ⬆️
trio/_util.py 100.00% <100.00%> (ø)
trio/testing/_sequencer.py 100.00% <100.00%> (ø)
trio/tests/test_ssl.py 99.85% <100.00%> (+0.56%) ⬆️
trio/tests/test_subprocess.py 100.00% <100.00%> (ø)
trio/_core/tests/test_ki.py 90.12% <0.00%> (-9.88%) ⬇️
trio/_highlevel_ssl_helpers.py 100.00% <0.00%> (+11.76%) ⬆️

@agronholm agronholm marked this pull request as ready for review January 24, 2022 22:00
@pquentin
Copy link
Member

While it makes sense to stop depending on async_generator at runtime, surely we should continue making sure in tests that we support it correctly for projects that are still using it?

@agronholm
Copy link
Contributor Author

I suppose so. I will restore the tests.

@agronholm
Copy link
Contributor Author

While it makes sense to stop depending on async_generator at runtime, surely we should continue making sure in tests that we support it correctly for projects that are still using it?

We cannot drop async_generator and continue to support it at the same time. That is the inconvenient truth. Here is the line that makes all the difference.

@agronholm
Copy link
Contributor Author

I think we need a change in strategy: Deprecate async_generator itself first by emitting a deprecation warning on import. After a sufficient amount of time, we can drop the support in trio.

@pquentin
Copy link
Member

Agreed. In any case this means we can close this pull request.

@pquentin pquentin closed this Jan 26, 2022
@pquentin pquentin deleted the drop_async_generator branch January 26, 2022 13:19
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 this pull request may close these issues.

2 participants