Enable savepints for all engines. Add disable_savepoints param #45
Conversation
|
I like this PR although I am not using this library against SQLite on Python < 3.6. |
There was a problem hiding this comment.
What happens if you're on Python 3.5 and try to use savepoints?
According to the removed assertion in the unit test, previously you'd get a TypeError at the point where you called transaction.savepoint().
If the new code can do the same, I'd say we can merge it (but we should keep the unit test with that assertion, in case SQLITE_NO_SAVEPOINT is True).
| @@ -147,11 +146,7 @@ def savepoint(self): | |||
| # support savepoints but Postgres is whitelisted independent of its | |||
| # version. Possibly additional version information should be taken | |||
| # into account (ajung) | |||
There was a problem hiding this comment.
This comment is outdated now you're no longer checking engine names.
|
@mgedmin If not and your database/driver does not support subtransactions you get a I'm reluctant to try and make a decision automagically as the logic required will likely be incomplete and error prone. "Explicit is better than implicit" and all that. Python 2.7 and 3.5 sqlite who are using savepoints (likely optimistic ones) should either:
According to https://devguide.python.org/#status-of-python-branches Python 3.5 end-of-life is September this year. Perhaps it's enough to just clearly document this and call it out in the changelog? |
|
I think documenting is enough. |
|
What exactly is missing to get this pull request to continue? I just had to add an ugly workaround in my app because of this - and I would like to get rid of it soon. :-) |
|
I am closing this old PR. Feel free to re-open it if you want to work on it. |
Fixes #44. Looking for feedback but requires documentation before merging.
From my Travis tests it appears Python 3.6 was the first to ship with a SQLite version that supports savepoints. (It's possible to hack in support using pysqlite for earlier versions using: https://github.com/lrowe/pysqlite/wiki/Building-pysqlite-with-static-env-and-savepoints.)
Should this be considered a breaking change? Potentially those running older versions of SQLite may now need to add
disable_savepoints=Trueto their register call.