Enable savepints for all engines. Add disable_savepoints param

[lrowe]

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=True to their register call.

[icemac]

I like this PR although I am not using this library against SQLite on Python < 3.6.
I'd consider this a breaking change as we should not yet drop support for Python < 3.6.

[mgedmin]

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).

[mgedmin]

This comment is outdated now you're no longer checking engine names.

[lrowe]

@mgedmin transaction.savepoint() without optimistic=True raises a TypeError when one of its data managers is missing a savepoint method. To get that same behaviour you would need to use zope.sqlalchemy.register(disable_savepoints=True).

If not and your database/driver does not support subtransactions you get a sqlalchemy.exc.OperationalError when the database errors on the transaction.commit() or .rollback() having called transaction.savepoint() in the transaction. Example test failures before I added the version based unittest.skipIf: https://travis-ci.org/github/zopefoundation/zope.sqlalchemy/jobs/668966939

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:

• stay on zope.sqlalchemy 1.3
• specify disable_savepoints=True
• install pysqlite >= 2.8.0 to get working savepoints

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?

[icemac]

I think documenting is enough.

[dwt]

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. :-)

[sallner]

@dwt We need the following:

• Rebase to master
• Add documentation of the non-support in readme and changelog
• Fix comment in code.

@lrowe Do you intend to work on that soon, or should someone else step in?

[icemac]

I am closing this old PR. Feel free to re-open it if you want to work on it.