Skip to content

Async generators always ensure that inner generator are closed properly #230

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

Conversation

leszekhanusz
Copy link
Collaborator

@leszekhanusz leszekhanusz commented Aug 16, 2021

Previously, when using break inside async for session.susbcribe(..., the inner generator will not close properly because of the _generator variable reference which is still available from the session.

This PR will try to always ensure that if an async generator closes, the inner generators will also close.

This should ensure that a subscribe will close directly after a break if you don't keep any references of the generator

@codecov-commenter
Copy link

Codecov Report

Merging #230 (23ab7a2) into master (4e08f09) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master      #230   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           16        16           
  Lines         1064      1067    +3     
=========================================
+ Hits          1064      1067    +3     
Impacted Files Coverage Δ
gql/client.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4e08f09...23ab7a2. Read the comment docs.

@leszekhanusz leszekhanusz merged commit 66174a6 into graphql-python:master Aug 16, 2021
@leszekhanusz leszekhanusz deleted the fix_break_should_stop_async_generator branch November 8, 2021 15:38
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