bpo-27645: Supporting native backup facility of SQLite by lelit · Pull Request #4238 · python/cpython (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation79 Commits52 Checks0 Files changed
Conversation
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
[ Show hidden characters]({{ revealButtonHref }})
Suggested by Aviv Palivoda.
Suggested by Aviv Palivoda.
When sqlite3_backup_step() returns OK it means that it was able to do it's work but there are further pages to be copied: in such case it's better to immediately start the next iteration, without opening a window that allows other threads to go on and possibly locking the database.
Suggested by Aviv Palivoda.
…llback
Add new SQLITE_DONE integer constant to the module, that could be used by the callback to determine whether the backup is terminated.
This should address Aviv's concerns about how the sqlite3_backup_step() errors are handled.
This is needed for SQLite < 3.8.7.
Supporting native backup facility of SQLite bpo-27645: Supporting native backup facility of SQLite
Compilation fails on Windows, due to misdeclaration of unlink(). I call that function to remove an incomplete backup when an error happens: I looked around in the sources and couldn't figure out a portable way to do that. Any hint?
| #ifdef HAVE_UNISTD_H |
|---|
| #include <unistd.h> |
| #else |
| extern int unlink(const char *); |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This won't work under Windows. You have to use DeleteFileW, see posixmodule.c
Thanks for making the requested changes!
@berkerpeksag, @tiran: please review the changes made to this pull request.
Is there anything left to do on my part?
Will this be merged before 3.7b1?
@tiran and @berkerpeksag, looks this PR is waiting on your reviews of changes. Is there anything else holding this up? It seems like a lot of work went into this PR over a long period and it is quite self-contained. If everyone agrees it is ready to be merged into master in the next few days and if the buildbots are happy with it, I might be persuaded to allow it into 3.7.0b3 as well.
Sorry, I've been busy with family stuff and I will take a look at this PR today or tomorrow.
This looks pretty good to me. I've rebased @lelit's branch, made some cosmetic changes, and fixed a segfault (824d5f8)
About supporting a more recent SQLite version as minimum: Last time I've used a newer API (I documented the minimum SQLite version) we got several reports saying "we can't compile Python with SQLite X.Y.Z anymore." Perhaps we can use the death of Python 2.7 as an opportunity and get rid of most of the compatibility code in Python 3.8 or Python 3.9?
If everyone agrees it is ready to be merged into master in the next few days and if the buildbots are happy with it, I might be persuaded to allow it into 3.7.0b3 as well.
I'm planning to merge this in an hour or two. I've changed the versionadded marker to 3.8 but should I revert it back to 3.7?
@berkerpeksag, Thanks for taking this on! Yeah, let's go crazy and change it to 3.7. We can always change it later if, for some reason, it doesn't make 3.7.
@ned-deily changed versionadded to 3.7 and added an entry to Doc/whatsnew/3.7.rst. What's the current process for getting something in after b1? Do I need to open a PR against the 3.7 branch or some special branch?
@berkerpeksag, you can just set the "need backport to 3.7" label to start an auto backport to 3.7
Thanks @lelit for the PR, and @berkerpeksag for merging it 🌮🎉.. I'm working now to backport this PR to: 3.7.
🐍🍒⛏🤖
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request
(cherry picked from commit d7aed41)
Co-authored-by: Emanuele Gaifas lelegaifax@gmail.com
@lelit thank you for your patience! I know that this was a frustrating experience for you, but I hope you are going to continue contributing to Python :)
berkerpeksag pushed a commit that referenced this pull request
(cherry picked from commit d7aed41)
Co-authored-by: Emanuele Gaifas lelegaifax@gmail.com
@berkerpeksag thank you and don't worry, I've been around for enough time to know how it goes and I will surely keep that for the remaining half of my life 😃
jo2y pushed a commit to jo2y/cpython that referenced this pull request
