depend on pyramid_retry and move the tween over the excview #55
Conversation
| activate_hook = maybe_resolve(activate_hook) | ||
| annotate_user = asbool(settings.get('tm.annotate_user', True)) | ||
|
|
||
| if 'tm.attempts' in settings: # pragma: no cover |
There was a problem hiding this comment.
I haven't checked pyramid_retry but does it support tm.attempts? Or should we link to documentation on how to configure pyramid_retry.
If someone is using tm.attempts and wants to stick to Pyramid <=1.7 they will have to use an older version of pyramid_tm. I am okay with that, just something we should document heavily.
There was a problem hiding this comment.
I shipped pyramid_retry 0.1 with support for tm.attempts but I changed my mind in 0.2 and now it only supports retry.attempts. This is so that tm.attempts is unused and I can properly warn people here to stop using it.
|
|
||
| from .compat import reraise | ||
| from .compat import binary_type, text_type | ||
| from .compat import text_ |
There was a problem hiding this comment.
I wonder if instead of carrying this compat around, we should just import it from pyramid.compat... but I have a feeling that is a larger change in the context of pyramid_*.
pyramid_tm/__init__.py
Outdated
| if request.tm._retryable(*exc_info[:-1]): | ||
| exc = exc_info[1] | ||
| if exc: | ||
| zope.interface.directlyProvides(exc, IRetryableError) |
There was a problem hiding this comment.
This is the best use of zope.interface I have seen.
There was a problem hiding this comment.
I just pushed a version that uses z.i.alsoProvides thanks to @davisagli catching the issue. Super subtle!
mmerickel
force-pushed
the
feature/split-retry
branch
from
86b0824
to
8321461
Compare
mmerickel
force-pushed
the
feature/split-retry
branch
from
8321461
to
80c9445
Compare
- pull retry logic out and support pyramid_retry if installed - place the tween over the excview
mmerickel
force-pushed
the
feature/split-retry
branch
from
80c9445
to
9922406
Compare
This PR removes attempt / retry support. As such the tm is now simpler and focuses only on handling exceptions. The tm is also moved above the excview so that exception views are managed by the transaction manager.
I have hopes that we will eventually move the response callbacks to a tween as well and then we would move the tm over them as well, but that is not the case in this PR.
I had a version of this that depended on pyramid_retry but it turned out that I could keep pyramid_tm working on older versions of pyramid if I didn't, and since it's a totally separate project now it actually works without the retry support anyway if you don't want it. A warning is emitted now if pyramid_retry is not installed and tm.attempts is set.
This is a breaking change that would be released as pyramid_tm 2.0.