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.

Sign up for GitHub

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

Jump to bottom

Bad connections should not be put back into the pool #195

Closed
thehesiod opened this issue Jul 1, 2019 · 2 comments
Closed

Bad connections should not be put back into the pool #195

thehesiod opened this issue Jul 1, 2019 · 2 comments

Comments

@thehesiod
Copy link
Contributor

Pool.acquire returns a _PoolConnectionContextManager.

After __aenter__ a pyodbc.OperationalError: ('08S01', '[08S01] [Microsoft][ODBC Driver 17 for SQL Server]Communication link failure (0) (SQLExecDirectW)') can be thrown in which case the connection may be permanently bad.

We now go to its __aexit__ where it unconditionally calls await self._pool.release(self._conn).

Pool.release at this point doesn't know if the connection is bad.

Instead if pyodbc.OperationalError is raised the connection should probably always be closed.
@thehesiod
Copy link
Contributor Author

btw hello @jettify on another aio project :]

thehesiod added a commit to thehesiod-forks/aioodbc that referenced this issue Jul 1, 2019
@thehesiod
fix for aio-libs#195
b55cda1
@thehesiod thehesiod mentioned this issue Jul 1, 2019
Merged
4 tasks
jettify pushed a commit that referenced this issue Jul 5, 2019
@thehesiod @jettify
fix for #195 (#196)
63f2a11 
* fix for #195

* add changes

* - fix fix :)
- add test
- make setup more resilient on OSX

* attempt to fix tests

* fixes

* test tweaks

* more test cleanup

* make tests besides my new one pass

* fix my test

* fix crashing

* reduce changes and fix potential issues

* better fix

* better fixes

* fix test hopefully

* bumps to try to fix crash

* wow is this test nuanced!

* flake fixes

* swap back and disable test

* fix test?

* fix tests
@aigulasan
Copy link

In my case . we used this "'exec sp_change_users_login 'AUTO_FIX',"cause DB was updated and for this user we made auto_fix

@jettify jettify closed this as completed Apr 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@jettify @thehesiod @aigulasan