Opened 17 years ago

Closed 17 years ago

Last modified 16 years ago

#144 closed defect (released)

Throw DatabaseFormatError instead of DatabaseOpeningError when database format is unsupported

Reported by: Richard Boulton Owned by: Richard Boulton
Priority: normal Milestone: 1.0.0
Component: Library API Version: SVN trunk
Severity: normal Keywords:
Cc: Blocked By:
Blocking: #118 Operating System: All

Description (last modified by Richard Boulton)

This was suggested by Fabrice Colin on xapian-discuss: if an application writer wants to distinguish a failure to open a database because of a change in the database format from other failures to open the database (so that automatic upgrading of databases can be implemented, for example), they currently have to parse the error message. It would be much more helpful if there was a separate error type to catch.

Fabrice Colin suggested the name "DatabaseVersionError", but I fear that this could be confused with the "DatabaseModifiedError". Instead, I suggest the name "DatabaseFormatError". It should be a subclass of "DatabaseOpeningError" - both so that existing code which catches DatabaseOpeningError will continue to work, and because it's an error to do with opening the database.

Attachments (1)

formaterror.patch (8.3 KB ) - added by Richard Boulton 17 years ago.
Patch which implements DatabaseFormatError

Download all attachments as: .zip

Change History (7)

comment:1 by Richard Boulton, 17 years ago

Owner: changed from Olly Betts to Richard Boulton

by Richard Boulton, 17 years ago

Attachment: formaterror.patch added

Patch which implements DatabaseFormatError

comment:2 by Richard Boulton, 17 years ago

Blocking: 118 added
Status: newassigned

Attached a patch which implements this feature.

I think it would be useful to apply this for 1.0.0, and it's not terribly invasive. The testsuite passes fully with it applied - which is both a good thing (obviously), and a bad thing (because I didn't have to change any of the tests, which implies that none of this code is being tested).

Olly - feel free to comment, and change to block #120 if you want. I think it would be a shame to make application writers have to jump through hoops to support the database format change, though - I think use of flint in the 0.9 series became quite widespread, judging from the mailing list.

comment:3 by Olly Betts, 17 years ago

This seems to have a high benefit/risk ratio, so I think it's suitable for 1.0.0.

DatabaseFormatError seems a poor name though - it immediately makes me think "there's an error in the format of the database", so confusing it with DatabaseCorruptError. I don't really see the confusion between DatabaseVersionError and DatabaseModifiedError myself - I guess you're thinking of the Btree versioning, but that's really at a lower level than users are generally aware of. Also, DatabaseModifiedError will be going away in the medium term. Perhaps DatabaseFormatVersionError is better, though that's getting a bit long - identifiers should be titles, not essays. I think my preference is DatabaseVersionError.

Also, the documentation comment says it means an "unknown format", which isn't the case really - to be precise, it means a version of a known format which we don't support. If you tried to open a sleepycat database or an sqlite database, you wouldn't get this error.

Perhaps we should have a simple feature test too? We could just include a few "empty" databases of old formats in the testsuite data.

comment:4 by Richard Boulton, 17 years ago

Resolution: fixed
Status: assignedclosed

I've applied the patch, changing the name to DatabaseVersionError - 2 votes to 1, and I'm not really fussed. :) Also adjusted the doccomment for the error class to say unsupported rather than unknown (as all the other documentation comments already did). Also adjusted the test case I added yesterday to test for DatabaseVersionError, and it seems to be working correctly. So, this should be fixed.

comment:5 by Olly Betts, 17 years ago

Operating System: All
Resolution: fixedreleased

Fixed in 1.0.0 release.

comment:7 by Richard Boulton, 16 years ago

Description: modified (diff)
Milestone: 1.0.0
Note: See TracTickets for help on using tickets.