Data manager's should_retry did not get called

[sirn]

I'm trying to use tm.attempts to automatically retry a query in case of exception and found that zone.sqlalchemy.SessionDataManager.should_retry did not get called at all in pyramid_tm. I've tried debugging the problem and found these lines as culprit:

try:
    manager.abort()
    retryable = manager._retryable(*exc_info[:-1])

As far as I understand, this is related to how manager._retryable works:

1. manager._retryable will get a list of data manager instances via manager.get()._resources and call that data manager's should_retry. (1):

   for dm in self.get()._resources:
       should_retry = getattr(dm, 'should_retry', None)
       if (should_retry is not None) and should_retry(error):
           return True

2. However calling manager.abort() will empty the manager.get()._resources.

3. Since manager._retryable is called after abort in pyramid_tm, it will try to iterate over an empty list and no data manager's should_retry gets called at all. 😢 (2):

   try:
       manager.abort()
       retryable = manager._retryable(*exc_info[:-1])
       if (number <= 0) or (not retryable):
           reraise(*exc_info)

This patch simply reversing these two lines since real Attempt in transaction.manager also has it reversed (3):

def _retry_or_raise(self, t, v, tb):
    retry = self.manager._retryable(t, v)
    self.manager.abort()
    if retry:

I'm not quite sure how to write a test for this one, so I added active to DummyTransaction to simulate data manager's presenceness to ensure _retryable is only returned if there's active data manager (tests properly failing after I swapped two lines in __init__.py back).

Thanks raydeo in #pyramid for helping me tracking this down! 👍

[mmerickel]

thanks for this