GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | |||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | |||||||||||
GeezerVison is showing github.com — gh-83638: Add sqlite3.Connection.autocommit for PEP 249 compliant behaviour by erlend-aasland · Pull Request #93823 · python/cpython Conversation
|
|
@zzzeek, @maggyero, I'd appreciate your review on this, if only by building and testing it, and/or taking a look at the added tests. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
@AlexWaygood would you mind taking a look at the doc changes? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
For external reviewers, here's some help on how to get started reviewing this PR:
|
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
cc. @bitdancer |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
A few small things, nothing major. (I only looked at the docs :)
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
Thanks, Alex! 🚀 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Thanks, I definitely welcome more eyes on this. Heads-up: you should take a look at the latest emails on the db-sig mailing list, so you have an up to date understanding of how the upcoming PEP 249 autocommit attribute will look. EDIT: I mixed up the dates; since alpha 2 is scheduled for Monday, I'm going to aim for that with this PR. If you don't have time to review this now, @maggyero, feel free to do a post-merge review. If we need to change the implementation, we can do so all the way until the feature freeze in May (IIRC). |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Co-authored-by: C.A.M. Gerlach <[email protected]>
There was an error while loading. Please reload this page.
Co-authored-by: C.A.M. Gerlach <[email protected]>
Thanks for the information and sorry for the delay, restoring the whole commit mode context took me longer than I expected. I actually did a complete rewriting of a post of mine on Stack Overflow on this topic to have a clear mental model.
Yes, I did. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
No problem. I'll merge this tomorrow evening (CET) so I'm sure we get this into the next alpha. If you manage to do a review by then; good. If not, I'd be happy with a post-merge review, if you are fine with that. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
This PR is great! Here is my review of the implementation, which is the most urgent. I’ll try to review the documentation this afternoon before the evening merge.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
To everyone involved here and on the issue(s), thanks for fruitful discussions and helpful reviews. I'll handle the eventual deprecation of ´isolation_mode´ in a follow-up issue/PR; I'll close gh-83638 with this PR. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
I added an extra change request in the tests.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
@maggyero, I'm landing this now. If you have further remarks, comment on the issue and we'll do a follow-up PR. Thanks for the review! |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
And here is the documentation review. =)
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
Thank you very much for your work and patience @erlend-aasland and the reviewers, I think you did an amazing job! This will improve the user experience significantly. According to the SQLite engine documentation, SQLite is the most widely deployed database engine and might even be the single most widely deployed software component of any type, so the SQLite3 driver for Python has to be of top quality. I am fully convinced by the design (thanks @malemburg), implementation and tests. The documentation looks good to me, and we could address the remaining suggested changes in a follow-up documentation PR. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
Add an autocommit property to sqlite3.Connection with a PEP 249 compliant manual commit mode and migrate
There was an error while loading. Please reload this page.