[issue45512] [sqlite3] simplify "isolation level"

[Erlend E. Aasland]

Erlend E. Aasland  added the comment:

Thanks for reviews and improvements, Dong-hee :)

--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue45512] [sqlite3] simplify "isolation level"

[Dong-hee Na]

Dong-hee Na  added the comment:


New changeset 0920b61a0cb30128287ebafab1df8cad3a3dffdb by Erlend Egeberg 
Aasland in branch 'main':
bpo-45512: Use Argument Clinic to set sqlite3 isolation level (GH-29593)
https://github.com/python/cpython/commit/0920b61a0cb30128287ebafab1df8cad3a3dffdb


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue45512] [sqlite3] simplify "isolation level"

[Erlend E. Aasland]

Change by Erlend E. Aasland :


--
pull_requests: +27836
pull_request: https://github.com/python/cpython/pull/29593

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue45512] [sqlite3] simplify "isolation level"

[Erlend E. Aasland]

Erlend E. Aasland  added the comment:

> Now we close this issue right?

Not yet ;) I have an AC tweak I'd like to add. Wait for a PR anytime soon.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue45512] [sqlite3] simplify "isolation level"

[Dong-hee Na]

Dong-hee Na  added the comment:

@erlendaasland

Now we close this issue right?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue45512] [sqlite3] simplify "isolation level"

[Dong-hee Na]

Dong-hee Na  added the comment:


New changeset e002bbc6cce637171fb2b1391ffeca8643a13843 by Dong-hee Na in branch 
'main':
bpo-45512: Simplify manage isolation level (GH-29562)
https://github.com/python/cpython/commit/e002bbc6cce637171fb2b1391ffeca8643a13843


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue45512] [sqlite3] simplify "isolation level"

[Dong-hee Na]

Dong-hee Na  added the comment:


New changeset 5f9247e36a0213b0dcfd43533db5cf6570895cfd by Erlend Egeberg 
Aasland in branch 'main':
bpo-45512: Extend `sqlite3` test suite regarding isolation levels (GH-29576)
https://github.com/python/cpython/commit/5f9247e36a0213b0dcfd43533db5cf6570895cfd


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue45512] [sqlite3] simplify "isolation level"

[Erlend E. Aasland]

Change by Erlend E. Aasland :


--
pull_requests: +27820
pull_request: https://github.com/python/cpython/pull/29576

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue45512] [sqlite3] simplify "isolation level"

[Dong-hee Na]

Change by Dong-hee Na :


--
pull_requests: +27810
stage: resolved -> patch review
pull_request: https://github.com/python/cpython/pull/29562

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue45512] [sqlite3] simplify "isolation level"

[Dong-hee Na]

Dong-hee Na  added the comment:


New changeset 822c3dcce3996e411c1ff5c432c6ac7d2845cfd6 by Erlend Egeberg 
Aasland in branch 'main':
bpo-45512: Raise exception if sqlite3.Connection.__init__ is called with bad 
isolation level (#29561)
https://github.com/python/cpython/commit/822c3dcce3996e411c1ff5c432c6ac7d2845cfd6


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue45512] [sqlite3] simplify "isolation level"

[Erlend E. Aasland]

Erlend E. Aasland  added the comment:

Need to amend GH-29053, so I'm opening until the fix is merged.

--
resolution: fixed -> 
status: closed -> open

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue45512] [sqlite3] simplify "isolation level"

[Erlend E. Aasland]

Change by Erlend E. Aasland :


--
pull_requests: +27809
pull_request: https://github.com/python/cpython/pull/29561

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue45512] [sqlite3] simplify "isolation level"

[Erlend E. Aasland]

Change by Erlend E. Aasland :


--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue45512] [sqlite3] simplify "isolation level"

[Dong-hee Na]

Dong-hee Na  added the comment:


New changeset b567b9d74bd9e476a3027335873bb0508d6e450f by Erlend Egeberg 
Aasland in branch 'main':
bpo-45512: Simplify isolation_level handling in `sqlite3` (GH-29053)
https://github.com/python/cpython/commit/b567b9d74bd9e476a3027335873bb0508d6e450f


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue45512] [sqlite3] simplify "isolation level"

[Erlend E. Aasland]

Erlend E. Aasland  added the comment:

IMO, GH-29053 improves readability, mainly because of these factors:

- Argument Clinic now takes care of the ref count dance, and as a bonus, it is 
more self-documenting (stating clearly that it accepts str and None)
- Begin statement validation has been refactored out, which IMO helps 
understanding how isolation level and begin statement is related
- The isolation level setter is now greatly simplified; it fits well in a 
screenful, there are fewer ref count dances
- Using get_begin_statement() is much less noisier than the previous string 
compare code in pysqlite_connection_set_isolation_level()


The only thing that still bugs me is the "magical" 6 byte offset needed to 
convert between isolation level and begin statement. Perhaps a set of 
conversion functions would make it more readable.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue45512] [sqlite3] simplify "isolation level"

[Dong-hee Na]

Dong-hee Na  added the comment:

Hmm, now I understood what you intended,
The code was hard to read without knowing the condition of the begin_statement.
Would this be a trade-off in terms of code readability?

--
nosy: +corona10

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue45512] [sqlite3] simplify "isolation level"

[Erlend E. Aasland]

Change by Erlend E. Aasland :


--
keywords: +patch
pull_requests: +27325
stage:  -> patch review
pull_request: https://github.com/python/cpython/pull/29053

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue45512] [sqlite3] simplify "isolation level"

[Erlend E. Aasland]

New submission from Erlend E. Aasland :

While working on bpo-45126 / GH-28227 (and while working on the AC conversion, 
bpo-40956), I've been slightly frustrated on the implementation of sqlite3 
"isolation level". The code is messy, and we've got two connection members that 
carry pretty much the same type of information (self->isolation_level, and 
self->begin_statement).

I would like to make the following enhancements:
  - merge 'isolation_level' and 'begin_statement' members in some kind of way
  - split pysqlite_connection_set_isolation_level():
+ one method for parsing and storing the 'isolation_level' member
+ one method for carrying out any needed SQLite API operation

This should result in a cleaner connection __init__() method.


Another slightly related performance enhancement could be to cache the "begin" 
(and "commit") statements as sqlite3_stmt pointers on the connection object, 
but that is a digression.

--
components: Extension Modules
messages: 404182
nosy: erlendaasland
priority: low
severity: normal
status: open
title: [sqlite3] simplify "isolation level"
versions: Python 3.11

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com