classification
Title: dbm.open(..., flag="n") raises dbm.error if file exists and is rejected by whichdb
Type: behavior Stage: commit review
Components: Library (Lib) Versions: Python 3.3, Python 3.2, Python 3.1, Python 2.7
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: brian.curtin Nosy List: brian.curtin, denversc, eric.araujo, python-dev
Priority: normal Keywords: patch

Created on 2011-03-14 06:42 by denversc, last changed 2011-03-14 21:51 by brian.curtin.

Files
File name Uploaded Description Edit
dbm_open_n_flag_error_invalid_file_fix_v1.patch denversc, 2011-03-14 06:42 Proposed Fix (version 1) review
issue11491.diff brian.curtin, 2011-03-14 17:40 review
Messages (7)
msg130791 - (view) Author: Denver Coneybeare (denversc) * Date: 2011-03-14 06:42
dbm.open() with flag="n" raises dbm.error if the given file exists but whichdb doesn't recognize it.  In the documentation for dbm.open() the "n" flag is documented to "Always create a new, empty database, open for reading and writing".  To me, this implies that if the file exists it will unconditionally be overwritten with a newly-created database, irrespective of its contents.

The code below illustrates a scenario (and indeed the scenario that I ran into) where dbm.open(..., flag="n") will throw dbm.error when it should just blow away the existing file and create a new, empty database:

import dbm
open("test.db", "w").close() # create empty file
dbm.open("test.db", flag="n")

The cause of the exception is that within dbm.open() there is a call to whichdb to determine the file type.  The fix would be to skip this whichdb check if the "n" flag is specified.

I don't think that this change will cause backward compatibility issues, since I find it hard to believe that existing applications would rely on this exception being raised in this scenario.  However, to *guarantee* no compatibility break an alternate fix could leave the current behavior of the "n" flag and introduce a new flag, perhaps "o" for "overwrite", with this "unconditional overwrite" behavior.

A proposed patch is attached: dbm_open_n_flag_error_invalid_file_fix_v1.patch
msg130865 - (view) Author: Brian Curtin (brian.curtin) * (Python committer) Date: 2011-03-14 17:40
Attached is a slightly updated version of the patch. If the assertEqual for any reason were to fail, the file wouldn't be closed, leading to a ResourceWarning. That'll work on 3.2+, but if this is backported consideration will have to be made there (try/finally, probably).

Otherwise, in concept I think this patch is alright.
msg130869 - (view) Author: Denver Coneybeare (denversc) * Date: 2011-03-14 18:21
Looks good to me.  I thought the same thing about the file not being closed on error, but all of the other tests in the file also suffer from that problem, so I just followed the convention set out by the other tests.  Maybe if you eventually commit this change, you could improve this behavior in the other tests as well.
msg130871 - (view) Author: √Čric Araujo (eric.araujo) * (Python committer) Date: 2011-03-14 18:28
Fortunately, addCleanup exists in 2.7 and 3.1.
msg130879 - (view) Author: Roundup Robot (python-dev) Date: 2011-03-14 19:36
New changeset f8d603a2a0af by briancurtin in branch '3.1':
Fix #11491. When dbm.open was called with a file which already exists and
http://hg.python.org/cpython/rev/f8d603a2a0af
msg130893 - (view) Author: Roundup Robot (python-dev) Date: 2011-03-14 20:36
New changeset ec8d64396be9 by briancurtin in branch '3.2':
Fix #11491. When dbm.open was called with a file which already exists and
http://hg.python.org/cpython/rev/ec8d64396be9

New changeset 0515206e36ed by briancurtin in branch 'default':
Fix #11491. When dbm.open was called with a file which already exists and
http://hg.python.org/cpython/rev/0515206e36ed
msg130914 - (view) Author: Brian Curtin (brian.curtin) * (Python committer) Date: 2011-03-14 21:51
The 3.x side of things is taken care of. Still need to see if there is any issue on 2.7 where things are organized differently and dbm is in C.
History
Date User Action Args
2011-03-14 21:51:53brian.curtinsetassignee: brian.curtin
messages: + msg130914
nosy: eric.araujo, brian.curtin, denversc, python-dev
stage: commit review
2011-03-14 20:36:53python-devsetnosy: eric.araujo, brian.curtin, denversc, python-dev
messages: + msg130893
2011-03-14 19:36:09python-devsetnosy: + python-dev
messages: + msg130879
2011-03-14 18:28:52eric.araujosetnosy: + eric.araujo
messages: + msg130871
2011-03-14 18:21:01denverscsetnosy: brian.curtin, denversc
messages: + msg130869
2011-03-14 17:40:27brian.curtinsetfiles: + issue11491.diff
nosy: brian.curtin, denversc
messages: + msg130865
2011-03-14 15:27:53denverscsetnosy: + brian.curtin

versions: + Python 3.1, Python 2.7, Python 3.2
2011-03-14 06:42:42denversccreate