On Wed, Mar 28, 2012 at 6:37 PM, Chris McDonough <chr...@plope.com> wrote: > On Wed, 2012-03-28 at 17:06 -0400, Jim Fulton wrote: >> On Wed, Mar 28, 2012 at 4:37 PM, Chris McDonough <chr...@plope.com> wrote: >> > On Wed, 2012-03-28 at 14:21 -0400, Jim Fulton wrote: >> ... >> > A decorator for running some code in the context of a txn and retrying >> > retryable exceptions would be nice higher level behavior. Â I'd be >> > willing to do this work over this weekend. >> >> Cool. Don't forget the transaction note part. :) >> Too many transactions in our apps don't have >> notes, especially transactions that happen >> outside of web requests. >> >> > In the meantime, I think the existing attempts context manager still >> > needs the small fix I proposed in my original message. Â Can you confirm >> > that my understanding of the its intent seems roughly correct? >> >> I didn't see a statement of intent. I think your fix us good, >> but the code (that I wrote and your fix) makes my head hurt. :) >> >> I'd say, make some test cases for the bug and make it pass. > > OK. Once I fix this "Attempts" bug, I think the decorator code is just > a higher level interface that uses it: > > class job(object): > def __init__(self, attempts=1, note=None, manager=None): > self.attempts = attempts > self.note = note > if manager is None: > manager = transaction.manager > self.manager = manager > > def __call__(self, wrapped): > note = self.note > if note is None: > note = getattr(wrapped, '__name__', None) > def inner(*arg, **kw): > for attempt in self.manager.attempts(self.attempts): > with attempt as t: > t.note(note) > return wrapped(*arg, **kw) > > .. or something like that...
It could be written that way, although I would use a much simpler implementation. The attempt design was a reach to overcome the limitations of the with statement. I'm not at all happy with it, although I couldn't think of anything better at the time. I hate to build on it. >> I'll admit, I didn't understand "Either that or it needs to not try to >> do a commit itself, and leave it up to the caller." > > "The caller" in this case is the body of the context manager which > "calls" __exit__ by raising an exception or returning. I just meant > instead of: > > for attempt in transaction.attempts(3): > with attempt as t: > .. do stuff, rely on context manager __exit__ to commit .. > > You might change its contract to: > > for attempt in transaction.attempts(3): > with attempt as t: > .. do stuff .. > t.commit() # you must commit yourself, __exit_ catches > # errors only and aborts on exception > > But this would break existing code as well as probably going against the > spirit of intent. Yup. Jim -- Jim Fulton http://www.linkedin.com/in/jimfulton _______________________________________________ For more information about ZODB, see http://zodb.org/ ZODB-Dev mailing list - ZODB-Dev@zope.org https://mail.zope.org/mailman/listinfo/zodb-dev
