GeezerVison is showing github.com — Document sqlite3.connect() as implicitly opening transactions in the new PEP-249 manual commit mode · Issue #99824 · python/cpython
Document sqlite3.connect() as implicitly opening transactions in the new PEP-249 manual commit mode #99824DescriptionDocumentation
In sqlite3, @malemburg specified that connect(), commit(), and rollback() implicitly open transactions in the new PEP-249 manual commit mode implemented in PR #93823:
If this is set to False, the module could then implement the
correct way of handling transactions, which means:
a) start a new transaction when the connection is opened
b) start a new transaction after .commit() and .rollback()
c) don't start new transactions anywhere else
d) run an implicit .rollback() when the connection closes
So I expect to see that information clearly documented.
Yet the PR documented it only for commit() and rollback() (item b), not for connect() (item a):
* :mod:`!sqlite3` ensures that a transaction is always open,
so :meth:`Connection.commit` and :meth:`Connection.rollback`
will implicitly open a new transaction immediately after closing
the pending one.
To me it is important to document it also for connect() rather than relying on user’s deduction, which is not obvious at all since for instance in legacy manual commit mode, sqlite3 does not implicitly open transactions when connect() is called but when execute() is called on a DML statement.
Linked PRs
Activityerlend-aasland commented on Nov 27, 2022 Thanks for creating an issue and explaining your reasoning behind this proposed change.
In sqlite3, @malemburg specified [...]
For your information; the PEP 249 is the specification, and Marc-André has recently updated it with information about the autocommit attribute:
https://peps.python.org/pep-0249/#autocommit erlend-aasland commented on Nov 27, 2022
[...] Yet the PR documented it only for commit() and rollback() (item b), not for connect() (item a):
* :mod:`!sqlite3` ensures that a transaction is always open,
so :meth:`Connection.commit` and :meth:`Connection.rollback`
will implicitly open a new transaction immediately after closing
the pending one.
To me it is important to document it also for connect() rather than relying on user’s deduction for connect(), which is not obvious at all since for instance in legacy manual commit mode, sqlite3 does not implicitly open transactions when connect() is called but when execute() is called on a DML statement.
The docs you point to are the docs explaining transaction control using the autocommit attribute. This section is only relevant if you've got a Connection object, so I'm not sure this is the best place (in the docs) to add a sentence about connect() behaviour. If we want to explain the behaviour of sqlite3.connect's autocommit parameter, we should do so in that part of the reference1.
geryogam commented on Nov 28, 2022 ContributorAuthor More actions Yes we should probably document the implicit transaction opening behaviour of the connect() function in Reference > Module functions > sqlite3.connect, like we did for the Connection.commit() and Connection.rollback() methods in Reference > Connection objects > sqlite3.Connection.commit and Reference > Connection objects > sqlite3.Connection.rollback:
commit()
Commit any pending transaction to the database. If autocommit is True, or there is no open transaction, this method does nothing. If autocommit is False, a new transaction is implicitly opened if a pending transaction was committed by this method.
rollback()
Roll back to the start of any pending transaction. If autocommit is True, or there is no open transaction, this method does nothing. If autocommit is False, a new transaction is implicitly opened if a pending transaction was rolled back by this method.
But we should also probably document it in Explanation > Transaction control since it is a general section about transaction control. And as you said, when the autocommit parameter is False a transaction is implicitly opened for connect(), Connection.commit() and Connection.rollback(), whereas when the Connection.autocommit attribute is False a transaction is implicitly opened only for Connection.commit() and Connection.rollback() since the Connection instance is already created. So the autocommit parameter and the Connection.autocommit attribute are actually two different ways of controlling transactions. What about renaming the ‘Transaction control via the autocommit attribute’ subsection to ‘Transaction control via the autocommit parameter or attribute’, or simply to ‘Transaction control via autocommit’. Likewise for the ‘Transaction control via the isolation_level attribute’ subsection? erlend-aasland commented on Nov 28, 2022
But we should also probably document it in Explanation > Transaction control since it is a general section about transaction control.
I'm -1 for that change; I'm afraid you are overcomplicating things. We should keep the docs (both reference and explanation, but especially the former) to the point; we should resist the urge to be overly verbose.
IMO, the right thing to do, is to update the sqlite3.connect() reference only.
So the autocommit parameter and the Connection.autocommit attribute are actually two different ways of controlling transactions [...]
No, they are the same; the connect autocommit parameter only provides a way to set the initial value of the Connection.autocommit attribute.
What about renaming the ‘Transaction control via the autocommit attribute’ subsection to ‘Transaction control via the autocommit parameter or attribute’, or simply to ‘Transaction control via autocommit’. Likewise for the ‘Transaction control via the isolation_level attribute’ subsection?
I'm -1 for such a change; the initial value of both attributes can be set using a keyword parameter when the connection object is set up. This is all explained in the reference; please take a look at the existing docs. There is no need to be overly verbose. geryogam commented on Nov 28, 2022 ContributorAuthor More actions
IMO, the right thing to do, is to update the sqlite3.connect() reference only.
Done. added a commit that references this issue on Nov 30, 2022gh-99824: Document that sqlite3.connect implicitly open a transaction…
MetadataMetadataStatusShow more project fields
DevelopmentNo branches or pull requestsIssue actions
|