FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

bpo-45243: Add support for setting/getting `sqlite3` connection limits by erlend-aasland · Pull Request #28463 · python/cpython · GitHub

Repository navigation

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

Merged
pablogsal merged 16 commits into
python:mainfrom
erlend-aasland:sqlite-limits
Nov 1, 2021
Merged

pablogsal merged 16 commits into
python:mainfrom
erlend-aasland:sqlite-limits

Conversation

erlend-aasland commented Sep 19, 2021 •
edited by bedevere-bot
Loading

Copy link
Copy Markdown
Contributor

erlend-aasland commented Oct 7, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

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.

Comment thread Modules/_sqlite/connection.c Outdated
Comment thread Modules/_sqlite/connection.c Outdated
Comment thread Modules/_sqlite/module.c

Copy link
Copy Markdown
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.

Copy link
Copy Markdown
Contributor Author

PTAL, @serhiy-storchaka.

Copy link
Copy Markdown
Contributor Author

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

Copy link
Copy Markdown
Contributor Author

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

pablogsal left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM

pablogsal merged commit b6b38a8 into python:main Nov 1, 2021
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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


Back | FazBrowse Home | New Git URL