bpo-27645: Supporting native backup facility of SQLite #4238
Conversation
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.
|
Compilation fails on Windows, due to misdeclaration of |
Modules/_sqlite/connection.c
Outdated
| #ifdef HAVE_UNISTD_H | ||
| #include <unistd.h> | ||
| #else | ||
| extern int unlink(const char *); |
There was a problem hiding this comment.
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? |
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 |
|
@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. |
|
GH-6064 is a backport of this pull request to the 3.7 branch. |
|
@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 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 😃 |
This is a new version of PR #377, rebased on more recent
masterto solve the conflicts on the oldNEWSfile.It implements a new requested feature since last review.
See https://bugs.python.org/issue27645
https://bugs.python.org/issue27645