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 ) > _______________________________________________ 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 )