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

bpo-45243: Add support for setting/getting sqlite3 connection limits #28463

Merged
merged 16 commits into from
Nov 1, 2021

Conversation

erlend-aasland
Copy link
Contributor

@erlend-aasland erlend-aasland commented Sep 19, 2021

@erlend-aasland
Copy link
Contributor Author

erlend-aasland commented Oct 7, 2021

Thinking about this, I wonder if connection limits are better implemented as attributes:

import sqlite3
cx = sqlite3.connect(":memory:")
lim = cx.SQLITE_LIMIT_LENGTH
cx.SQLITE_LIMIT_LENGTH = 100

# instead of
lim = cx.getlimit(sqlite3.SQLITE_LIMIT_LENGTH)
cx.setlimit(sqlite3.SQLITE_LIMIT_LENGTH, 100)

I added GH-28790 as an alternative implementation.

@erlend-aasland
Copy link
Contributor Author

@serhiy-storchaka I changed the argument spec:

  • setlimit(limit, value, /) => setlimit(category, limit, /)
  • getlimit(limit, /) => getlimit(category, /)

I believe this is clearer and more aligned with the SQLite documentation.

@erlend-aasland
Copy link
Contributor Author

PTAL, @serhiy-storchaka.

@erlend-aasland
Copy link
Contributor Author

@serhiy-storchaka I believe all comments have been resolved. Would you like to take another look at this PR?

@erlend-aasland
Copy link
Contributor Author

@pablogsal would you mind taking a look at this? I believe I've addressed all of Serhiy's comments.

Copy link
Member

@pablogsal pablogsal left a comment

Choose a reason for hiding this comment

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

LGTM

@pablogsal pablogsal merged commit b6b38a8 into python:main Nov 1, 2021
@erlend-aasland erlend-aasland deleted the sqlite-limits branch November 1, 2021 22:53
JelleZijlstra added a commit to JelleZijlstra/typeshed that referenced this pull request Apr 15, 2022
- Blob from python/cpython#30680 (and anticipating that python/cpython#91550 will be merged)
- Aggregate window functions from python/cpython#20903
- Serialize/deserialize from python/cpython#26728
- Limit setting from python/cpython#28463
srittau pushed a commit to python/typeshed that referenced this pull request Apr 16, 2022
- Blob from python/cpython#30680 (and anticipating that python/cpython#91550 will be merged)
- Aggregate window functions from python/cpython#20903
- Serialize/deserialize from python/cpython#26728
- Limit setting from python/cpython#28463
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.

5 participants