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

459.bugfix #459

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

459.bugfix #459

wants to merge 3 commits into from

Conversation

summatix
Copy link

What do these changes do?

When running in a pool, upon close, fixes the error:

An open stream object is being garbage collected; call "stream.close()" explicitly.

@summatix summatix changed the title pr.bugfix 459.bugfix Dec 22, 2019
@prryplatypus
Copy link

Just bumping this since I seem to still have this issue 6 months after the PR was opened. Any progress done at all?

@summatix
Copy link
Author

summatix commented Jun 6, 2020

This works. The PR was just never merged.

@prryplatypus
Copy link

Gonna be the annoying guy that tags @webknjaz to ask if he can have a look then, pls :)

@webknjaz
Copy link
Member

webknjaz commented Jun 6, 2020

I'm not the best person to review this. So I cannot accept this since I don't know if this design decision (which is effectively extending the public API) is desired. But what I can tell you is that I'd not merge this w/o any tests proving that this works.

@webknjaz webknjaz closed this Jun 6, 2020
@webknjaz webknjaz reopened this Jun 6, 2020
@webknjaz webknjaz requested review from jettify and terricain June 6, 2020 22:18
@codecov
Copy link

codecov bot commented Jun 6, 2020

Codecov Report

Merging #459 (1dad293) into master (196e325) will decrease coverage by 1.76%.
The diff coverage is 85.71%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #459      +/-   ##
==========================================
- Coverage   92.66%   90.90%   -1.77%     
==========================================
  Files          35       35              
  Lines        5413     5419       +6     
  Branches      584      585       +1     
==========================================
- Hits         5016     4926      -90     
- Misses        309      399      +90     
- Partials       88       94       +6     
Flag Coverage Δ
ubuntu-latest_3.10_mariadb-10.2 89.44% <85.71%> (-0.01%) ⬇️
ubuntu-latest_3.10_mariadb-10.3 89.44% <85.71%> (-0.01%) ⬇️
ubuntu-latest_3.10_mariadb-10.4 89.44% <85.71%> (-0.01%) ⬇️
ubuntu-latest_3.10_mariadb-10.5 89.42% <85.71%> (-0.01%) ⬇️
ubuntu-latest_3.10_mariadb-10.6 89.42% <85.71%> (-0.01%) ⬇️
ubuntu-latest_3.10_mariadb-10.7 89.42% <85.71%> (-0.01%) ⬇️
ubuntu-latest_3.10_mysql-5.7 90.14% <85.71%> (-0.01%) ⬇️
ubuntu-latest_3.10_mysql-8.0 ?
ubuntu-latest_3.7_mariadb-10.2 88.85% <85.71%> (-0.01%) ⬇️
ubuntu-latest_3.7_mariadb-10.3 88.85% <85.71%> (-0.01%) ⬇️
ubuntu-latest_3.7_mariadb-10.4 88.85% <85.71%> (-0.01%) ⬇️
ubuntu-latest_3.7_mariadb-10.5 88.83% <85.71%> (-0.01%) ⬇️
ubuntu-latest_3.7_mariadb-10.6 88.83% <85.71%> (-0.01%) ⬇️
ubuntu-latest_3.7_mariadb-10.7 88.83% <85.71%> (-0.01%) ⬇️
ubuntu-latest_3.7_mysql-5.7 89.60% <85.71%> (-0.01%) ⬇️
ubuntu-latest_3.7_mysql-8.0 ?
ubuntu-latest_3.8_mariadb-10.2 89.48% <85.71%> (-0.01%) ⬇️
ubuntu-latest_3.8_mariadb-10.3 89.48% <85.71%> (-0.01%) ⬇️
ubuntu-latest_3.8_mariadb-10.4 89.48% <85.71%> (-0.01%) ⬇️
ubuntu-latest_3.8_mariadb-10.5 89.46% <85.71%> (-0.01%) ⬇️
ubuntu-latest_3.8_mariadb-10.6 89.46% <85.71%> (-0.01%) ⬇️
ubuntu-latest_3.8_mariadb-10.7 89.46% <85.71%> (-0.01%) ⬇️
ubuntu-latest_3.8_mysql-5.7 90.18% <85.71%> (-0.01%) ⬇️
ubuntu-latest_3.8_mysql-8.0 ?
ubuntu-latest_3.9_mariadb-10.2 89.35% <85.71%> (-0.01%) ⬇️
ubuntu-latest_3.9_mariadb-10.3 89.35% <85.71%> (-0.01%) ⬇️
ubuntu-latest_3.9_mariadb-10.4 89.35% <85.71%> (-0.01%) ⬇️
ubuntu-latest_3.9_mariadb-10.5 89.33% <85.71%> (-0.01%) ⬇️
ubuntu-latest_3.9_mariadb-10.6 89.33% <85.71%> (-0.01%) ⬇️
ubuntu-latest_3.9_mariadb-10.7 89.33% <85.71%> (-0.01%) ⬇️
ubuntu-latest_3.9_mysql-5.7 90.05% <85.71%> (-0.01%) ⬇️
ubuntu-latest_3.9_mysql-8.0 ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
aiomysql/connection.py 78.59% <83.33%> (-5.96%) ⬇️
aiomysql/pool.py 95.18% <100.00%> (-1.81%) ⬇️
tests/test_sha_connection.py 38.63% <0.00%> (-61.37%) ⬇️
tests/conftest.py 81.90% <0.00%> (-5.03%) ⬇️
tests/test_connection.py 97.34% <0.00%> (-2.66%) ⬇️

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 196e325...1dad293. Read the comment docs.

@webknjaz
Copy link
Member

webknjaz commented Jun 6, 2020

FWIW I've restarted the CI and it fails.

@summatix
Copy link
Author

summatix commented Jun 9, 2020

This is quite clearly a bug, not a design choice.

I no longer use this library and so I no longer have interest in pushing this PR forward. My fork is available to those who encounter this bug.

@RazzerDE
Copy link

This is quite clearly a bug, not a design choice.

I no longer use this library and so I no longer have interest in pushing this PR forward. My fork is available to those who encounter this bug.

Did you found a better, working library for async MySQL? I really need one and this library is just broken.

@aio-libs aio-libs deleted a comment from CLAassistant Jan 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants