Code review comment for lp://staging/~free.ekanayaka/storm/tpc-support

Revision history for this message
Stuart Bishop (stub) wrote :

On Tue, Mar 6, 2012 at 5:28 PM, Pippo Paperino <email address hidden> wrote:

> I see, but if your backend does not support TPC, then
> transaction.commit() will fail, because if you enable TPC on the store
> with zstorm.set_default_tpc('my-store', False), then the data manager
> will try to call begin()/prepare(), which would fail if the backend does
> not support that.

I see. You are correct.

> Why? ZStorm is already supporting mixing TPC and non-TPC stores via the
> set_default_tpc() method? And that's from the very first version of the
> branch in this MP.

> I still I don't understand why you are saying this, as mentioned above
> this possible with the set_default_tpc() method, which works per-store,
> unless I'm missing something

The confusion is mine sorry. In my mind I had set_default_tpc() as a
method on the ZStorm utility.

I am happy with this branch. It is bigger than I thought it would be,
but it all seems necessary and the test coverage is great. Thanks!

The only additional thing I think we want is in the tests' setUp or
tearDown (or both), is to use your recover() method to discover and
rollback any prepared transactions left by failed tests. It doesn't
need to be on this branch though.

--
Stuart Bishop <email address hidden>

« Back to merge proposal