[issue16958] The sqlite3 context manager does not work with isolation_level=None
aaugustin added the comment: * Thesis * I belive that using the connection as a context manager is an inadequate API for controlling transactions because it's very likely to result in subtly broken code. As a consequence, my recommendation would be to deprecate this API. * Argumentation * If you nest a naive transaction context manager (BEGIN / COMMIT / ROLLBACK), you'll get very lousy transaction semantics. Look at this example: with connection: # outer transaction with connection: # inner transaction do_X_in_db() do_Y_in_db() # once in a while, you get an exception there... With this code, when you get an exception, X will be presevred in the database, but not Y. Most likely this breaks the expectations of the outer transaction. Now, imagine the inner transaction in deep inside a third-party library, and you understand that this API is a Gun Pointed At Feet. Of course, you could say don't nest, but: - this clashes with the expected semantics of Python context managers, - it's unreasonable to expect Python users to audit all their lower level libraries for this issue! Now, let's look at how popular database-oriented libraires handle this. SQLAlchemy provides an explicit begin() method: http://docs.sqlalchemy.org/en/latest/core/connections.html#sqlalchemy.engine.Connection.begin It also provides variants for nested transactions and two-phase commits. Django provide an all-purpose atomic() context manager: https://docs.djangoproject.com/en/stable/topics/db/transactions/#django.db.transaction.atomic That function takes a keyword argument, `savepoint`, to control whether a savepoint is emitted for nested transactions. So it's possible to implement a safe, nestable context manager with savepoints. However: - you need to provide more control, and as a consequence you cannot simply use the connection as a context manager anymore; - it takes a lot of rather complex code. See Django's implementation for an example: https://github.com/django/django/blob/stable/1.6.x/django/db/transaction.py#L199-L372 If you ignore the cross-database compatibility stuff, you're probably still looking at over a hundred lines of very stateful code... That's why I believe it's better to leave this up to user code, and to stop providing an API that looks good for trivial use cases but that's likely to introduce subtle transactional integrity bugs. -- nosy: +aaugustin ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue16958 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue10740] sqlite3 module breaks transactions and potentially corrupts data
aaugustin added the comment: On the other hand there is absolutely nothing broken on the implicit BEGIN (which is required by dbapi2 specification) nor on the isolation_level property which controls it. Those shouldn't be touched; there is no reason to. Nothing broken... unless one considers there should be a relation between the name of a parameter and the feature it controls ;-) `isolation_level` should really be called `transaction_mode`. It's a specific extension of the BEGIN TRANSACTION statement in SQLite and it's unrelated to the standard concept of transaction isolation levels. SQLite almost always operates at the SERIALIZABLE isolation level. (For exceptions to this rule, search for PRAGMA read_uncommitted; in the documentation.) This is a consequence of its file-lock based implementation of transactional atomicity. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue10740 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue10740] sqlite3 module breaks transactions and potentially corrupts data
aaugustin added the comment: Hey -- maintainer of Django's transaction support here. This ticket was brought to my attention again today. As I know a few things about this issue and I see Python core devs asking for input, I'll give my $0.02. The core of this issue is that, **by default**, the sqlite3 module performs a simplistic parsing of SQL queries and decides to commit before anything it doesn't understand, for example SAVEPOINT foo. I don't think it's a good idea to change sqlite's default operation mode to autocommit. PEP 249's prescription for transaction management aren't practical (and Django enforces the opposite behavior by default) but that's beyond the point. It's way too much backwards incompatible. However, I don't think it's a good idea either to automatically send a COMMIT when I want to make a SAVEPOINT. In fact, if you want to use transactions with sqlite, connection.isolation_level = None is almost warranted -- and then you do everything manually. For a slightly longer explanation, see https://www.youtube.com/watch?v=09tM18_st4I#t=1751 I've struggled mightily with all this when rewriting Django's transaction management. See: - https://github.com/django/django/blob/3becac84/django/db/backends/sqlite3/base.py#L107 https://github.com/django/django/blob/3becac84/django/db/backends/sqlite3/base.py#L398-L403 - https://github.com/django/django/blob/3becac84/django/db/transaction.py#L185-L195 I have implemented the workarounds I need and I we won't be able to remove them from Django anytime soon. As a consequence, I have little interest in getting this fixed. Still, it would be sane to stop committing before savepoints by default -- that kinda ruins the concept of savepoints :-) It would be even saner to stop parsing SQL. Does http://hg.python.org/cpython/file/85fd955c6fc8/Modules/_sqlite/cursor.c#l32 look like an adequate parser for http://sqlite.org/lang.html? Unfortunately, I don't have backwards-compatible proposal to fix this. Trying to account for a bit more syntax will help in the short term but not fix the underlying issue. PS: I find it unfair to brand SQLite as a neutered database engine. Its transaction model is vastly superior to, say, MySQL. Too bad sqlite3 ruins it. -- nosy: +aaugustin ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue10740 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue10740] sqlite3 module breaks transactions and potentially corrupts data
aaugustin added the comment: That patch solves the problem, at the cost of introducing an unwieldy API, operation_needs_transaction_callback. I'm very skeptical of the other API, in_transaction. Other database backends usually provide an autocommit attribute. autocommit is the opposite of in_transaction for all practical purposes. There's only two situations where they may be equal: - before the first query - after an explicit commit Then you aren't in a transaction and you aren't in autocommit. But in these cases, in practice, the question you want to ask is is the next query going to create a transaction? (and if not, I may want to create one.) So the semantic of connection.autocommit is much more useful than the semantic of connection.in_transaction. While you're there, it would be cool to provide connection.autocommit = True as an API to enable autocommit, because connection.isolation_level = None isn't a good API at all -- it's very obscure and has nothing to do with isolation level whatsoever. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue10740 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11553] Docs for: import, packages, site.py, .pth files
aaugustin aymeric.augus...@polyconseil.fr added the comment: I noticed an inconsistency in the docs, and I think it falls in the Language Reference section of this ticket. In the definition of the import statement, after: | from module import * we should add: | from relative_module import * It is also possible to replace this line by: | from (module | relative_module) import * -- nosy: +aaugustin ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11553 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11140] Error in the documentation of threading.Lock().release()
New submission from aaugustin aymeric.augus...@polyconseil.fr: The docs for the threading module state that: The release() method should only be called in the locked state; it changes the state to unlocked and returns immediately. If an attempt is made to release an unlocked lock, a RuntimeError will be raised. However, I noticed that catching RuntimeError does not work. Actually release() raises thread.error, which inherits directly Exception. I reproduced the behavior shown below in Python 2.6.6, 2.7.1 from MacPorts on Mac OS 10.6 and in Python 2.6.6 on Linux. The same happens in Python 3.2rc2 on Mac OS 10.6, except the thread module has been renamed to _thread so the exception becomes a _thread.error. import threading try: ... threading.Lock().release() ... except RuntimeError: ... pass ... Traceback (most recent call last): File stdin, line 2, in module thread.error: release unlocked lock try: ... threading.Lock().release() ... except Exception, e: ... print type(e) ... print type(e).mro() ... class 'thread.error' [class 'thread.error', type 'exceptions.Exception', type 'exceptions.BaseException', type 'object'] I do not know if this must be fixed in the docs or the code. Currently, the type of the exception is probably platform-dependant since the thread module is not provided on all platforms. -- components: Library (Lib) messages: 128128 nosy: aaugustin priority: normal severity: normal status: open title: Error in the documentation of threading.Lock().release() versions: Python 2.6, Python 2.7 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11140 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11140] threading.Lock().release() raises _thread.error, not RuntimeError
aaugustin aymeric.augus...@polyconseil.fr added the comment: I agree with your solution. Unfortunately, I do not know how you would redefine ThreadError to extend RuntimeError in a C extension. _thread.error is created line 741 in trunk/Modules/threadmodule.c: ThreadError = PyErr_NewException(thread.error, NULL, NULL); dummy_thread.error is created lin 21 in trunk/Lib/dummy_thread.py: class error(Exception): ... Changing this to RuntimeException is trivial. I don't think there are other implementations of threading in the Python source but I'd appreciate if someone more familiar with the interpreter could confirm. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11140 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
