| 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) *  |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) *  |
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) * |
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 |
|
|