classification
Title: Don't expose sqlite3 Cache and Statement
Type: behavior Stage: resolved
Components: Extension Modules Versions: Python 3.8
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: berker.peksag, ghaering, izbyshev, palaviv, r.david.murray
Priority: normal Keywords:

Created on 2017-05-03 19:52 by palaviv, last changed 2019-05-09 18:10 by berker.peksag. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 1440 merged palaviv, 2017-05-03 19:55
PR 1573 closed palaviv, 2017-05-13 16:58
Messages (14)
msg292936 - (view) Author: Aviv Palivoda (palaviv) * Date: 2017-05-03 19:52
Both the Cache and Statement objects are internally used and should not be exposed to the user from the sqlite3 module. They are not mentioned in the documentation as well.
msg293104 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2017-05-05 13:17
If these objects have been exposed in the past, we won't simply delete them.  At a minimum there would need to be a deprecation period, but is there a real motivation for deleting them?
msg293105 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2017-05-05 13:18
Sorry, by "real motivation" I meant something beyond just cleaning up the API...that's a real motivation, it may just not be enough.
msg293109 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2017-05-05 14:21
Even if users somehow managed to create Cache and Statement objects themselves, they are basically implementation details of the module and there is no way to use them to mess with the internal state of the module via using the current API (e.g. Cursor.statement is not exposed)
msg293128 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2017-05-05 17:17
That's the same motivation, not a new one :)  Someone somewhere may be using them for something, they've been around for a long time.  I hope not, though.
msg293134 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2017-05-05 18:46
> Someone somewhere may be using them for something, they've been around for a long time.

Well, you can use the same argument for every issue on the tracker. People can even rely on real bugs that are still open for 10 years, but that doesn't mean they shouldn't be fixed.

In this case, your argument looks a bit hypothetical to me. I can't think of valid usages of these objects (I understand people can abuse Python's dynamic nature, but still... :)) You've obviously spent much more time working on sqlite3 than me so I wonder how can they can be useful to third party users :) (e.g. can someone use them for some reason to test their enhanced version of pydoc's extension module support?)
msg293144 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2017-05-05 21:57
No, I'm arguing purely from a generic backward compatibility perspective.  There does not seem to be me be sufficient benefit to removing them to justify doing it.
msg293166 - (view) Author: Aviv Palivoda (palaviv) * Date: 2017-05-06 17:04
I have 3 argument for the motivation for this change:
1. Cleaner API - Both objects are internal implementation details. I think that we can look on exposing them as a bug.
2. Easier development - When you remove these objects from the API you can change them without any concern. I for one think that I can make the sqlite3 module faster by changing them from Python objects to simple C structs.
3. Documentation - Currently both objects are part of the API. Thus they should be documented but how would you document the Statement class? Do we really want to have a generic Cache class in the sqlite3 module?

I have less experience with cpython then you. Do you think that maybe the Cache class should be moved to more appropriate place (maybe collections) and be used by others?

> At a minimum there would need to be a deprecation period, 

Is there a place that document the deprecation process? I will update the patch.
msg293170 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2017-05-06 19:45
They are not part of the API, that is why they are not documented.  The convention of "always" using _ prefixed names for non-API stuff is (relatively) recent.  It used to be we just didn't document the non-API stuff.

Your second argument is a good motivation.  Let's see what others think.

I thought the deprecation process was documented in a PEP, but I can't find it.  Basically, we introduce a deprecation warning (and a document the deprecation, but that isn't needed here) in the next release, and then the release after that we actually do the removal.  Or in many cases we don't do the removal at all, we just leave the deprecation warning in place until 2.7 is out of maintenance (but I don't think we need to worry about that in this case).
msg293242 - (view) Author: Aviv Palivoda (palaviv) * Date: 2017-05-08 17:21
I am not sure how to raise the deprecation waning in this case. We use both the Cache and Statement objects as part of the sqlite3 module internal flow. How can I only warn the user when he creates this classes directly and not when the sqlite3 module uses them?
msg293257 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2017-05-08 22:26
I don't do much with the C API, but since your goal is to remove them from the PyMODINIT_FUNC, I would think you could replace those entries with calls to wrapper functions that issue the deprecation and then call the real function.
msg293619 - (view) Author: Aviv Palivoda (palaviv) * Date: 2017-05-13 17:01
I opened a separate PR for the deprecation of the Cache and Statement.  

> I would think you could replace those entries with calls to wrapper functions that issue the deprecation and then call the real function.

I made wrapper deprecation classes that issue the deprecation. I am not sure if there is a easier way...
msg325446 - (view) Author: Alexey Izbyshev (izbyshev) * (Python triager) Date: 2018-09-15 16:36
Some additional motivation for removing Cache: it may be used to crash the interpreter (#31734).
msg341989 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2019-05-09 18:06
New changeset e6576248e5174ca5daa362cfd610c07e7eb3a2ae by Berker Peksag (Aviv Palivoda) in branch 'master':
bpo-30262: Don't expose private objects in sqlite3 (GH-1440)
https://github.com/python/cpython/commit/e6576248e5174ca5daa362cfd610c07e7eb3a2ae
History
Date User Action Args
2019-05-09 18:10:30berker.peksagsetstatus: open -> closed
stage: patch review -> resolved
resolution: fixed
versions: + Python 3.8, - Python 3.7
2019-05-09 18:06:11berker.peksagsetmessages: + msg341989
2018-09-15 16:36:48izbyshevsetnosy: + izbyshev
messages: + msg325446
2017-05-13 17:01:30palavivsetmessages: + msg293619
2017-05-13 16:58:46palavivsetpull_requests: + pull_request1668
2017-05-08 22:26:49r.david.murraysetmessages: + msg293257
2017-05-08 17:21:38palavivsetmessages: + msg293242
2017-05-06 19:45:52r.david.murraysetmessages: + msg293170
2017-05-06 17:04:18palavivsetmessages: + msg293166
2017-05-05 21:57:14r.david.murraysetmessages: + msg293144
2017-05-05 18:46:53berker.peksagsetmessages: + msg293134
2017-05-05 17:17:13r.david.murraysetmessages: + msg293128
2017-05-05 14:21:17berker.peksagsetmessages: + msg293109
stage: patch review
2017-05-05 13:18:19r.david.murraysetmessages: + msg293105
2017-05-05 13:17:33r.david.murraysetnosy: + r.david.murray
messages: + msg293104
2017-05-03 19:55:47palavivsetpull_requests: + pull_request1538
2017-05-03 19:52:10palavivcreate