Pylons/pyramid_tm — pull request #8 by sirn (2013-01-21 23:23:28+00:00)
Title: Data manager's should_retry did not get called

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:

```python
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](https://github.com/zopefoundation/transaction/blob/master/transaction/_manager.py#L143)):

```python
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. :cry: ([2](https://github.com/Pylons/pyramid_tm/blob/master/pyramid_tm/__init__.py#L79-L80)):

```python
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](https://github.com/zopefoundation/transaction/blob/master/transaction/_manager.py#L162)):

```python
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! :+1: