Skip to content

With + Connection - returns cursor rather than the connection, doesn't close connection on exit #735

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
Closed
greghoop opened this issue Oct 23, 2018 · 3 comments · Fixed by #886
Closed

With + Connection - returns cursor rather than the connection, doesn't close connection on exit #735

greghoop opened this issue Oct 23, 2018 · 3 comments · Fixed by #886

Comments

@greghoop
Copy link

greghoop commented Oct 23, 2018 •
edited
Loading

By returning a cursor rather than a connection, extra code needs to be written to handle exceptions and close the connection. Why not return a connection object, then a user can use a second nested with to get a cursor object. This way, wherever an error occurs, the cursors/connections will be neatly closed, and tracebacks will nicely propagate.

I noticed this was a problem when using SSH, and the tunnel wasn't getting decommissioned because the DB connection was still active. Other users also having this problem:

https://stackoverflow.com/questions/31214658/can-i-use-pymysql-connect-with-with-statement

Relevant lines in the code:

PyMySQL/pymysql/connections.py

Line 494 in 09040ab

def __enter__(self):

PyMySQL/pymysql/connections.py

Line 498 in 09040ab

def __exit__(self, exc, value, traceback):

@methane
Copy link
Member

methane commented Oct 23, 2018

These APIs are added for compatibility with mysqlclient.
And these APIs are deprecated in mysqlclient later.

https://github.com/PyMySQL/mysqlclient-python/blob/9fb618806bf638fa67f40b9cc3dafc4779237b63/MySQLdb/connections.py#L278-L290

But I forgot to deprecate these API in PyMySQL.
Wait at least two years: one year for deprecation, next one year for removal.

@methane methane mentioned this issue Nov 2, 2018
Merged
@methane methane mentioned this issue Jan 19, 2019
Closed
@jvperrin jvperrin mentioned this issue Jan 21, 2019
Open
@mark-schaal
Copy link

Glad to know I'm not alone in seeing this, and +1 to @greghoop link to https://stackoverflow.com/questions/31214658/can-i-use-pymysql-connect-with-with-statement.

The context manager is a good interim workaround if you do not want to assign the connection to a variable manually.

@methane methane mentioned this issue Dec 10, 2019
Merged
@methane methane removed the LongTerm label Jul 18, 2020
@methane methane mentioned this issue Sep 9, 2020
Merged
@hlzm
Copy link

hlzm commented Nov 2, 2020

I also got this strange issue, will this be solved soon?

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 3, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add context manager to Connection.
4 participants
@methane @hlzm @mark-schaal @greghoop