Ticket #144 (closed defect: released)

Opened 20 months ago

Last modified 9 months ago

Throw DatabaseFormatError instead of DatabaseOpeningError when database format is unsupported

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

Description (last modified by richard) (diff)

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

formaterror.patch (8.3 kB) - added by richard 20 months ago.
Patch which implements DatabaseFormatError?

Change History

Changed 20 months ago by richard

  • owner changed from olly to richard

Changed 20 months ago by richard

Patch which implements DatabaseFormatError?

Changed 20 months ago by richard

  • status changed from new to assigned
  • blocking set to 118

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.

Changed 20 months ago by olly

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.

Changed 20 months ago by richard

  • status changed from assigned to closed
  • resolution set to fixed

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.

Changed 20 months ago by olly

  • resolution changed from fixed to released

Fixed in 1.0.0 release.

Changed 20 months ago by trac

  • platform set to All

Changed 9 months ago by richard

  • description modified (diff)
  • milestone set to 1.0.0
Note: See TracTickets for help on using tickets.