GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — bpo-45613: Set `sqlite3.threadsafety` dynamically by erlend-aasland · Pull Request #29227 · python/cpython

Conversation

@erlend-aasland

@erlend-aasland erlend-aasland commented Oct 26, 2021 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

Use the compile-time selected default SQLite threaded mode to set the
DB-API 2.0 attribute 'threadsafety' during sqlite3 module initialisation.

SQLite to DB-API mappings:

  • SQLITE_THREADSAFE=0 => threadsafety=0 (single-thread mode)
  • SQLITE_THREADSAFE=1 => threadsafety=3 (fully threadsafe)
  • SQLITE_THREADSAFE=2 => threadsafety=1 (threadsafe at module level)

https://bugs.python.org/issue45613

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Erlend E. Aasland added 2 commits October 26, 2021 14:26
Erlend E. Aasland
bpo-45613: Dynamically set sqlite3.threadsafety
bf9677c 
Use the compile-time selected default SQLite threaded mode to set the
DB-API 2.0 attribute 'threadsafety'

Mappings:
  - SQLITE_THREADSAFE=0 => threadsafety=0
  - SQLITE_THREADSAFE=1 => threadsafety=3
  - SQLITE_THREADSAFE=2 => threadsafety=1
Erlend E. Aasland
Add NEWS and What's New
9747c80
@erlend-aasland
erlend-aasland requested a review from berkerpeksag as a code owner October 26, 2021 12:36
@the-knights-who-say-ni the-knights-who-say-ni added the CLA signed label Oct 26, 2021
@bedevere-bot bedevere-bot added the awaiting review label Oct 26, 2021
@erlend-aasland
erlend-aasland requested a review from malemburg October 26, 2021 12:36
@erlend-aasland

erlend-aasland commented Oct 26, 2021 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

On hold till #29129 is merged. Will rebase and update docs before marking this as ready for review.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
Erlend E. Aasland added 3 commits October 26, 2021 14:55
Erlend E. Aasland
Compatibility with older SQLite releases
9d2aa82
Erlend E. Aasland
Adjust wording: threaded => threading
06022ec
Erlend E. Aasland
Sync with main
4d8baff
@erlend-aasland
erlend-aasland marked this pull request as draft October 28, 2021 07:12
Erlend E. Aasland added 2 commits October 28, 2021 22:01
Erlend E. Aasland
Merge remote-tracking branch 'upstream/main' into sqlite-threadsafety
4ae65a0
Erlend E. Aasland
Update docs
4706e17
@erlend-aasland
erlend-aasland marked this pull request as ready for review October 28, 2021 20:46
Erlend E. Aasland added 3 commits October 28, 2021 22:51
Erlend E. Aasland
Nitpick
d45e27a
Erlend E. Aasland
Test for exact safety level
db01838
Erlend E. Aasland
Reword NEWS/What's New
396abd4
@erlend-aasland erlend-aasland added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Oct 29, 2021
@bedevere-bot

bedevere-bot commented Oct 29, 2021

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @erlend-aasland for commit 396abd4 🤖

If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Oct 29, 2021
@erlend-aasland

erlend-aasland commented Oct 29, 2021

Copy link
Copy Markdown
Contributor Author

Testing with buildbots to verify that this approach works across different SQLite versions/configurations. I've verified locally that it works with SQLite 3.7.15 (oldest supported), 3.36.0, and current 3.37.0 (dev).
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
Erlend E. Aasland added 5 commits October 30, 2021 00:27
Erlend E. Aasland
Merge branch 'main' into sqlite-threadsafety
2465ad2
Erlend E. Aasland
Merge branch 'main' into sqlite-threadsafety
da6481f
Erlend E. Aasland
Use sqlite3_treadsafe() instead of querying compile_options
084173a
Erlend E. Aasland
Adjust comments
abb7bae
Erlend E. Aasland
Bail if get_threadsafety() fails
686d70f
erlend-aasland
erlend-aasland commented Nov 2, 2021
View reviewed changes
Comment thread Modules/_sqlite/module.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
@erlend-aasland erlend-aasland added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Nov 2, 2021
@bedevere-bot

bedevere-bot commented Nov 2, 2021

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @erlend-aasland for commit 686d70f 🤖

If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Nov 2, 2021
erlend-aasland
erlend-aasland commented Nov 2, 2021
View reviewed changes
Comment thread Doc/library/sqlite3.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
malemburg
malemburg approved these changes Nov 3, 2021
View reviewed changes

@malemburg malemburg left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I think the patch is good to go as-is, without the additional changes you mentioned in the comments. Thanks, Erlend.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
erlend-aasland reacted with hooray emoji
All reactions
  • 🎉 1 reaction
Comment thread Doc/library/sqlite3.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Modules/_sqlite/module.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot added awaiting merge and removed awaiting review labels Nov 3, 2021
@erlend-aasland

erlend-aasland commented Nov 3, 2021 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

I think the patch is good to go as-is, without the additional changes you mentioned in the comments. Thanks, Erlend.

Thanks! There is one change I'd like to make before merging, though: add a versionchanged annotation to the threadsafety attribute doc.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
Erlend E. Aasland
Add version changed annotation
710a12e
@erlend-aasland
erlend-aasland requested a review from pablogsal November 3, 2021 19:59
pablogsal
pablogsal approved these changes Nov 3, 2021
View reviewed changes

@pablogsal pablogsal left a comment •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

LGTM

@malemburg do you have anything else you would like here to be addressed?

Edit: ah, I just saw that you approved before. I am landing this then

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
erlend-aasland reacted with hooray emoji
All reactions
  • 🎉 1 reaction
@pablogsal
pablogsal merged commit c273986 into python:main Nov 3, 2021
@bedevere-bot bedevere-bot removed the awaiting merge label Nov 3, 2021
@erlend-aasland
erlend-aasland deleted the sqlite-threadsafety branch November 3, 2021 21:01
@erlend-aasland

erlend-aasland commented Nov 3, 2021

Copy link
Copy Markdown
Contributor Author

Thanks, Marc-Andre & Pablo 🙏🏻
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@erlend-aasland erlend-aasland mentioned this pull request May 16, 2022
[sqlite3] set threadsafety attribute based on default SQLite threaded mode #89776
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@malemburg malemburg malemburg approved these changes
@pablogsal pablogsal pablogsal approved these changes
@berkerpeksag berkerpeksag Awaiting requested review from berkerpeksag

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

5 participants

@erlend-aasland @bedevere-bot @malemburg @pablogsal @the-knights-who-say-ni
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.