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

Use contextmanager for SQLite communication #91

Merged
merged 7 commits into from
Jan 22, 2024
Merged

Use contextmanager for SQLite communication #91

merged 7 commits into from
Jan 22, 2024

Conversation

hoxbro
Copy link
Member

@hoxbro hoxbro commented Dec 14, 2023

Fixes #8

Also adds a close method to close the database connection.

@ahuang11
Copy link
Contributor

I was wondering if you could simply call
with self.con to handle transactions?
https://blog.rtwilson.com/a-python-sqlite3-context-manager-gotcha/

(you probably know more about this than me)

Also, thoughts on using closing?
https://stackoverflow.com/questions/10029403/python-why-doesnt-writing-a-contextmanager-for-an-sqlite3-cursor-work

Copy link

codspeed-hq bot commented Dec 14, 2023

CodSpeed Performance Report

Merging #91 will not alter performance

Comparing use_context (1af7cc1) with main (1edf4ff)

Summary

✅ 6 untouched benchmarks

@hoxbro
Copy link
Member Author

hoxbro commented Dec 14, 2023

Nothing is set in stone, haven't really tested it yet.

Having to open a connection for each of the commit could be slow, but I don't know.

#8 had closing in in it, didn't use it here because I needed more functionality in the context manager.

@ahuang11
Copy link
Contributor

ahuang11 commented Dec 14, 2023

No I don't think it's opening a new connection each time; it's reusing the same connection with transactional things:
"""
Connection objects can be used as context managers that automatically commit or rollback transactions. In the event of an exception, the transaction is rolled back; otherwise, the transaction is committed.
"""
https://docs.python.org/3/library/sqlite3.html#how-to-use-the-connection-context-manager

@hoxbro
Copy link
Member Author

hoxbro commented Dec 14, 2023

There is a difference between the example in https://blog.rtwilson.com/a-python-sqlite3-context-manager-gotcha/ and https://docs.python.org/3/library/sqlite3.html#how-to-use-the-connection-context-manager.

The first creates a new connection in the context manager, and the latter uses an existing connection in the context manager, which can be used again. I suspect the latter does something similar to what I have done; it could be something only newer versions of Python support. I will check this tomorrow.

@hoxbro
Copy link
Member Author

hoxbro commented Dec 15, 2023

There is no significant difference between using self.con as a context manager and the implemented run_transaction. However, I have kept the run_transaction as it will use a cursor directly, which gives a bit of flexibility, like here (even though I think it is still possible by using self.con):

self.primary_key.validate(cursor.lastrowid, fields[self.primary_key.field_name])

@hoxbro hoxbro enabled auto-merge (squash) January 22, 2024 17:54
@hoxbro hoxbro merged commit 039754b into main Jan 22, 2024
16 checks passed
@hoxbro hoxbro deleted the use_context branch January 22, 2024 18:23
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.

Make connector.cursor a contextmanager
2 participants