This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: [sqlite3] cleanup callbacks (GIL handling, naming, ...)
Type: enhancement Stage: resolved
Components: Extension Modules Versions:
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: erlendaasland Nosy List: corona10, erlendaasland, miss-islington, petr.viktorin
Priority: normal Keywords: patch

Created on 2021-08-24 13:07 by erlendaasland, last changed 2022-04-11 14:59 by admin. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 27934 merged erlendaasland, 2021-08-24 18:21
PR 28088 merged erlendaasland, 2021-08-31 12:58
PR 28209 merged erlendaasland, 2021-09-07 15:26
Messages (9)
msg400207 - (view) Author: Erlend E. Aasland (erlendaasland) * (Python triager) Date: 2021-08-24 13:07
Quoting msg400205 by Petr in bpo-42064:
I think the module could use a more comprehensive review for GIL handling, rather than doing it piecewise in individual PRs. I recommend that any function passed to SQLite (and only those) should
  - be named `*_callback`, for clarity
  - acquire the GIL at the very start
  - release the GIL at the very end
msg400225 - (view) Author: Erlend E. Aasland (erlendaasland) * (Python triager) Date: 2021-08-24 18:31
I'd like to propose further enhancements:
  - use intermingled declarations in the affected functions; this will make GIL acquisition stand more out, and it also improves readability and makes it easier to trace refs
  - take the naming step further: I'd like to normalise PyObject * callback variable names, the extremely long function_pinboard_* names, and also drop the C callback prefixes ('_' and 'pysqlite_')

If you agree, I'll create separate PR's for those; one for each refactor. I think it will enhance readability a lot.
msg400265 - (view) Author: Petr Viktorin (petr.viktorin) * (Python committer) Date: 2021-08-25 13:40
The general policy is to only improve style for code you touch for some other reasons. You're changing lots of the callback code in bpo-42064, so I reckon making things more readable is fine.
msg400267 - (view) Author: Dong-hee Na (corona10) * (Python committer) Date: 2021-08-25 14:02
Obviously, this case could be code churn(we don't accept normally), but if other core-dev agree with the changes. I am fine.
msg400269 - (view) Author: Erlend E. Aasland (erlendaasland) * (Python triager) Date: 2021-08-25 14:06
We'll do the changes Petr proposed first, and then I'll see how invasive the other changes will be. If the diffs end up being concise, I'll put them up for review :)
msg400722 - (view) Author: miss-islington (miss-islington) Date: 2021-08-31 12:18
New changeset 001ef4600f5ab2e1d7825ddbc0f253377c234d7e by Erlend Egeberg Aasland in branch 'main':
bpo-44991: Make GIL handling more explicit in `sqlite3` callbacks (GH-27934)
https://github.com/python/cpython/commit/001ef4600f5ab2e1d7825ddbc0f253377c234d7e
msg401246 - (view) Author: Petr Viktorin (petr.viktorin) * (Python committer) Date: 2021-09-07 11:43
New changeset 0474d06008f8c9eba660ed20d336ffdc5c4db121 by Erlend Egeberg Aasland in branch 'main':
bpo-44991: Normalise `sqlite3` callback naming (GH-28088)
https://github.com/python/cpython/commit/0474d06008f8c9eba660ed20d336ffdc5c4db121
msg401247 - (view) Author: Petr Viktorin (petr.viktorin) * (Python committer) Date: 2021-09-07 11:44
The remaining function_pinboard_* renames are part of PR-27940.
msg403726 - (view) Author: Petr Viktorin (petr.viktorin) * (Python committer) Date: 2021-10-12 11:39
New changeset cfb1df3b71501a80ed57739181ec2ed30012c491 by Erlend Egeberg Aasland in branch 'main':
bpo-44991: Normalise function and collation callback naming (GH-28209)
https://github.com/python/cpython/commit/cfb1df3b71501a80ed57739181ec2ed30012c491
History
Date User Action Args
2022-04-11 14:59:49adminsetgithub: 89154
2021-10-12 11:39:20petr.viktorinsetmessages: + msg403726
2021-09-07 15:26:17erlendaaslandsetpull_requests: + pull_request26634
2021-09-07 13:06:38petr.viktorinsetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2021-09-07 11:44:42petr.viktorinsetmessages: + msg401247
2021-09-07 11:43:47petr.viktorinsetmessages: + msg401246
2021-08-31 12:58:16erlendaaslandsetpull_requests: + pull_request26531
2021-08-31 12:18:52miss-islingtonsetnosy: + miss-islington
messages: + msg400722
2021-08-25 14:06:47erlendaaslandsetmessages: + msg400269
2021-08-25 14:02:25corona10setnosy: + corona10
messages: + msg400267
2021-08-25 13:40:31petr.viktorinsetmessages: + msg400265
2021-08-24 18:31:48erlendaaslandsetmessages: + msg400225
title: [sqlite3] cleanup GIL handling -> [sqlite3] cleanup callbacks (GIL handling, naming, ...)
2021-08-24 18:21:26erlendaaslandsetkeywords: + patch
stage: patch review
pull_requests: + pull_request26383
2021-08-24 13:07:12erlendaaslandcreate