#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 )
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)
Change History (7)
comment:1 by , 18 years ago
Owner: | changed from | to
---|
by , 18 years ago
Attachment: | formaterror.patch added |
---|
comment:2 by , 18 years ago
Blocking: | 118 added |
---|---|
Status: | new → assigned |
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 , 18 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 , 18 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
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 , 18 years ago
Operating System: | → All |
---|---|
Resolution: | fixed → released |
Fixed in 1.0.0 release.
comment:7 by , 17 years ago
Description: | modified (diff) |
---|---|
Milestone: | → 1.0.0 |
Patch which implements DatabaseFormatError