Comment 4 for bug 132485

Revision history for this message
Richard Boulton (richardboulton) wrote :

Just to add my thoughts: adding only store.prepare() to the API makes sense to me; this keeps the API consistent and unsurprising since commit() is always used to finish the transaction. Also, changing code to use 2-phase commit instead of 1-phase is as simple as possible.

If 2-phase commit isn't supported by a database, prepare() should definitely raise an exception. Here's my reasoning:

 - the point of prepare() is to check that there are no constraint violations which will stop a commit() proceeding, to allow a coordinated 2-phase commit.
 - if it is possible for prepare() to return successfully without performing this check, it isn't possible to write a reliable 2-phase commit algorithm.
 - therefore, if prepare doesn't raise an exception if 2-phase commit isn't supported by the databases, programmers have to check whether 2-phase commit is supported by the underlying database if they want to know whether they're safe from database integrity violation.
 - this exposes programmers who haven't read the documentation carefully, or who are forgetful, to the risk that they're using a database backend which doesn't support 2-phase commit, but think that they _are_ using 2-phase commit, because prepare() is returning successfully.
 - this can lead to dataloss and general badness.

It is easy for a programmer to write a special case check around prepare() to catch an "Unsupported" exception (or, we could provide an optional parameter to prepare() to say "don't raise an exception if unsupported") - but we should make sure the programmer is aware that they're ignoring an error.

In simpler terms, I think programmers would find it surprising that prepare() could return successfully without actually doing the prepare, so the "principle of least surprise" says that it shouldn't do that.

To take the appropriate line from the Zen of Python "Errors should never pass silently."