On Tue, 2011-08-09 at 11:48 -0400, Zvezdan Petkovic wrote:

On Aug 8, 2011, at 4:44 PM, Chris McDonough wrote:

...

Kudos to whomever turned the "transaction" package's transaction manager into a context manager and was thoughtful enough to provide the "attempts" method (which returns a separate context manager, wrapping the txn context manager, and retries some number of configurable times). This makes writing code that integrates with a web system that commits or aborts as easy as:

import transaction

for attempt in transaction.attempts(attempts): with attempt as t: response = handler(request)

if t.isDoomed(): t.abort()

if commit_veto is not None: veto = commit_veto(request, response) veto and t.abort()

return response

Very nice.

- C

Yes, this is *very* convenient and I like to use it, but beware of the bug (it's not affecting you above because of the return).

According to the documentation, it's intended as a replacement for:

for i in range(3): try: with transaction: ... some something ... except SomeTransientException: continue else: break

So, presumably,

for attempt in transanction.attempts(retries): with attempt as tx: do_something()

should break after the *first* successful commit.

It doesn't. Instead, it commits do_something() ``retries`` times which is most probably not intended.

Not sure about that; I just return a value if it succeeds. I wouldnt expect it to know to break out of the loop without a return in there. I wound up with this, FWIW: def tm_tween_factory(handler, registry, transaction=transaction): # transaction parameterized for testing purposes commit_veto = registry.settings.get('pyramid_tm.commit_veto', default_commit_veto) attempts = int(registry.settings.get('pyramid_tm.attempts', 1)) if not commit_veto: commit_veto = None else: commit_veto = resolver.maybe_resolve(commit_veto) def tm_tween(request): if 'repoze.tm.active' in request.environ: # don't handle txn mgmt if repoze.tm is in the WSGI pipeline return handler(request) try: for attempt in transaction.attempts(attempts): with attempt as t: # make_body_seekable will copy wsgi.input if # necessary, otherwise it will rewind the copy # to position zero if attempts != 1: request.make_body_seekable() response = handler(request) if t.isDoomed(): raise AbortResponse(response) if commit_veto is not None: veto = commit_veto(request, response) if veto: raise AbortResponse(response) return response except AbortResponse, e: return e.response return tm_tween A little more verbose but still nice.

See https://bugs.launchpad.net/transaction/+bug/724332 The patch attached to the bug report is just a test that proves that behavior is unexpected and gives a workaround.

The workaround is simple enough and I prefer to use transaction.attempts convenience rather than the typical try-except code above.

Still, we really should find time to fix this in the transaction manager code so that it behaves as expected. (Or change the documentation and tests).

Regards,

Zvezdan

_______________________________________________ Zope-Dev maillist - Zope-Dev@zope.org https://mail.zope.org/mailman/listinfo/zope-dev ** No cross posts or HTML encoding! ** (Related lists - https://mail.zope.org/mailman/listinfo/zope-announce https://mail.zope.org/mailman/listinfo/zope )