Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dbm.open should be a context manager #63481

Closed
kousu mannequin opened this issue Oct 18, 2013 · 13 comments
Closed

dbm.open should be a context manager #63481

kousu mannequin opened this issue Oct 18, 2013 · 13 comments
Assignees
Labels
stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@kousu
Copy link
Mannequin

kousu mannequin commented Oct 18, 2013

BPO 19282
Nosy @rhettinger, @terryjreedy, @ncoghlan, @kousu, @PCManticore
Files
  • dbm.patch
  • dbm1.patch
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/ncoghlan'
    closed_at = <Date 2013-11-17.06:00:20.899>
    created_at = <Date 2013-10-18.07:45:10.904>
    labels = ['type-feature', 'library']
    title = 'dbm.open should be a context manager'
    updated_at = <Date 2014-03-08.17:54:18.487>
    user = 'https://github.com/kousu'

    bugs.python.org fields:

    activity = <Date 2014-03-08.17:54:18.487>
    actor = 'python-dev'
    assignee = 'ncoghlan'
    closed = True
    closed_date = <Date 2013-11-17.06:00:20.899>
    closer = 'python-dev'
    components = ['Library (Lib)']
    creation = <Date 2013-10-18.07:45:10.904>
    creator = 'kousu'
    dependencies = []
    files = ['32203', '32215']
    hgrepos = []
    issue_num = 19282
    keywords = ['patch']
    message_count = 13.0
    messages = ['200191', '200232', '200241', '200336', '200376', '200697', '201545', '201818', '203119', '203120', '203121', '203123', '212941']
    nosy_count = 7.0
    nosy_names = ['rhettinger', 'terry.reedy', 'ncoghlan', 'kousu', 'Arfrever', 'Claudiu.Popa', 'python-dev']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue19282'
    versions = ['Python 3.4']

    @kousu
    Copy link
    Mannequin Author

    kousu mannequin commented Oct 18, 2013

    This code doesn't work. I think it should.

    import dbm
    with dbm.open("what is box.db", "c") as db:
       db["Bpoind"] = Boing

    Indeed, there is nothing supporting PEP-343 for dbm on my system:
    [kousu@galleon ~]$ grep -r __exit__ /usr/lib/python3.3/dbm
    [kousu@galleon ~]$

    @kousu kousu mannequin added the stdlib Python modules in the Lib dir label Oct 18, 2013
    @PCManticore
    Copy link
    Mannequin

    PCManticore mannequin commented Oct 18, 2013

    Working on a patch for this.

    @serhiy-storchaka serhiy-storchaka added the type-feature A feature request or enhancement label Oct 18, 2013
    @PCManticore
    Copy link
    Mannequin

    PCManticore mannequin commented Oct 18, 2013

    Here's a patch.

    @terryjreedy
    Copy link
    Member

    I agree with the revised title. Test_context_manager() should also test that db is actually closed after the with statement. I presume you can use self.assertRaises and try to do something with db that will fail if it is properly closed.

            with self.assertRaises(<NotOpenError>):
                db['a'] = 'a'  # or whatever

    I haven't looked at the C code.

    @terryjreedy terryjreedy changed the title dbm is not a context manager dbm.open should be a context manager Oct 19, 2013
    @PCManticore
    Copy link
    Mannequin

    PCManticore mannequin commented Oct 19, 2013

    Attached patch checks that the db is actually closed after the with. Also, I noticed that dbm.dumb doesn't behave like the rest of the variants, as seen in the following:

    >>> import dbm.dumb as d
    >>> db = d.open('test.dat', 'c')
    >>> db.close()
    >>> db.keys()
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
      File "/tank/libs/cpython/Lib/dbm/dumb.py", line 212, in keys
        return list(self._index.keys())
    AttributeError: 'NoneType' object has no attribute 'keys'
    >>>

    vs

    >>> import dbm.gnu as g
    >>> db = g.open('test.dat', 'c')
    >>> db.close()
    >>> db.keys()
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
    _gdbm.error: GDBM object has already been closed
    >>>

    @rhettinger
    Copy link
    Contributor

    This seems like a reasonable request.

    @PCManticore
    Copy link
    Mannequin

    PCManticore mannequin commented Oct 28, 2013

    There is bpo-19385 for the inconsistency of dbm.dumb, with a patch.

    @ncoghlan
    Copy link
    Contributor

    Patch looks reasonable to me - if nobody beats me to it, I'll commit this before beta 1 :)

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Nov 17, 2013

    New changeset c2f1bb56760d by Nick Coghlan in branch 'default':
    Close bpo-19282: Native context management in dbm
    http://hg.python.org/cpython/rev/c2f1bb56760d

    @python-dev python-dev mannequin closed this as completed Nov 17, 2013
    @ncoghlan
    Copy link
    Contributor

    Thanks Claudiu!

    Additional tweaks in the committed version:

    • added a paragraph to the docs about the with statement support and moved the versionchanged note there
    • changed the example in the docs to use a with statement
    • added a basic test that the dumbdbm impl was actually closed, with a comment referencing bpo-19385 (since that isn't subject to the beta 1 deadline)

    @ncoghlan
    Copy link
    Contributor

    I also changed the signature of the enter/exit methods to just use PyObject * (rather than dbmobject *), since that allowed a bunch of casts to be removed and eliminated a couple of compiler warnings about type mismatches.

    @PCManticore
    Copy link
    Mannequin

    PCManticore mannequin commented Nov 17, 2013

    Cool, thanks!

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Mar 8, 2014

    New changeset 200207e50cbf by R David Murray in branch 'default':
    whatsnew: dbm.open is context manager. (bpo-19282)
    http://hg.python.org/cpython/rev/200207e50cbf

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    stdlib Python modules in the Lib dir type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    4 participants