gh-100414: Add SQLite backend to dbm#114481
Conversation
|
I suggest to focus this PR on the basic, required functionality only. We can optimise things in follow-up PRs. |
Sorry, something went wrong.
|
I think the basic functionality is ready for an initial round of reviews. TestsThere's a basic test suite in DocsI also added som basic docs; unless significant information is missing, I suggest we work on the docs in follow-up PRs. |
Sorry, something went wrong.
…ILD_TABLE never can raise a DB API exception
|
I made a core dev poll on Discourse regarding if the SQLite backend should be the default one; 16 out of 21 was in favour (Skip changed his mind post voting). This PR does not make the SQLite backend the default. If we want to change the default, we can do it post merging the feature itself. Another possibility, suggested by @ethanfurman:
@serhiy-storchaka, should we land this? |
Sorry, something went wrong.
|
It seems that currently it is not compatible with other dbm implementations. They all are bytes oriented, but accept strings as input (it is a Python 2 legacy). Most of the dbm tests use string input, so I expect that many user code does it too. But the output always expected to be bytes. Supporting other types, supported by Sqlite looks attractive, but we implement a backend to dbm, so it should be compatible. We can later add an option to alter the behavior, but it should not be default. |
Sorry, something went wrong.
As discussed in DM, I agree with this. I addressed your concern in e782fad. |
Sorry, something went wrong.
serhiy-storchaka
left a comment
There was a problem hiding this comment.
LGTM if all operations still accept strings.
Sorry, something went wrong.
|
With 34930cb, the dbm.sqlite3 backend now aligns with the other backends; for example, the general code snippet in the dbm docs now works as expected with the new backend. |
Sorry, something went wrong.
|
Let's land this, @serhiy-storchaka. There is plenty of opportunities to tweak the implementation and to expand the test suite. Thanks @smontanaro for the initial idea, @rhettinger for the initial implementation, and everyone else involved for ideas, reviews and critique. Thanks to @serhiy-storchaka for the, as always, extremely thorough review of the feature and the implementation. |
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as duplicate.
This comment was marked as duplicate.
Proposed fixes: |
Sorry, something went wrong.
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as resolved.
This comment was marked as resolved.
Co-authored-by: Raymond Hettinger <rhettinger@users.noreply.github.com> Co-authored-by: Serhiy Storchaka <storchaka@gmail.com> Co-authored-by: Mariusz Felisiak <felisiak.mariusz@gmail.com>
Initial version authored by Raymond in #100414 (comment).
📚 Documentation preview 📚: https://cpython-previews--114481.org.readthedocs.build/