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

fix for #195 #196

Merged
merged 20 commits into from Jul 5, 2019
Merged

fix for #195 #196

merged 20 commits into from Jul 5, 2019

Conversation

thehesiod
Copy link
Contributor

@thehesiod thehesiod commented Jul 1, 2019

What do these changes do?

Ensures we don't return bad connections back to the pool

Are there changes in behavior for the user?

Bad connections will no longer poison the pool

Related issue number

#195

Checklist

  • I think the code is well written
  • Unit tests for the changes exist
  • Documentation reflects the changes
  • Add a new news fragment into the CHANGES folder
    • name it <issue_id>.<type> (e.g. 588.bugfix)
    • if you don't have an issue_id change it to the pr id after creating the PR
    • ensure type is one of the following:
      • .feature: Signifying a new feature.
      • .bugfix: Signifying a bug fix.
      • .doc: Signifying a documentation improvement.
      • .removal: Signifying a deprecation or removal of public API.
      • .misc: A ticket has been closed, but it is not of interest to users.
    • Make sure to use full sentences with correct case and punctuation, for example: Fix issue with non-ascii contents in doctest text files.

@thehesiod
Copy link
Contributor Author

FYI: per https://www.python.org/dev/peps/pep-0249/#databaseerror, OperationalError means: https://www.python.org/dev/peps/pep-0249/#databaseerror so I think we're OK always closing the conn on one of these

@thehesiod
Copy link
Contributor Author

this is so frustrating trying to get these tests to pass, there's no reason why it keeps on failing to connect to the DB, argh

@lgtm-com
Copy link

lgtm-com bot commented Jul 2, 2019

This pull request introduces 1 alert when merging b7e9357 into 0124556 - view on LGTM.com

new alerts:

  • 1 for Unused import

@thehesiod
Copy link
Contributor Author

so summary:

  • running on jessie yields threads not existing: Error in my_thread_global_end(): 54 threads didn't exit. If I enable my test then a bunch more tests start failing saying that an exception was never raised.
  • running on stretch causes python to sporadically crash

between a rock and a hard place.

@thehesiod
Copy link
Contributor Author

ok getting closer, now some kind of docker container reaping issue

@thehesiod
Copy link
Contributor Author

done!

Copy link
Member

@jettify jettify left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@jettify jettify merged commit 63f2a11 into aio-libs:master Jul 5, 2019
@jettify
Copy link
Member

jettify commented Jul 6, 2019

New release available on PyPI https://pypi.org/project/aioodbc/0.3.3/

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.

None yet

2 participants