Hi Alberto,

Thanks!  I applied a slightly modified version of this patch:


The code can be checked out from http://svn.repoze.org/repoze.tm2/trunk/ .

Thanks again,

- C

Alberto Valverde wrote:
> Chris McDonough wrote:
>> (...)
>>>> There is an existing pattern for this if you can get a hold of the
>>>> transaction
>>>> object itself in app code:  transaction.doom()
>>> This is good for the cases where the middleware stacked below repoze.tm
>>> knows about the transaction, but worthless otherwise.
>> Any app code can doom the current transaction, actually by an import
>> and a couple of function calls:
>> import transaction
>> transaction.get().doom()
>> "t" is a transaction.Transaction object then representing the current
>> transaction.  It is effectively a "thread local" (although it's not a
>> Python threading.local, as the transaction package was created before
>> that existed).
> I was thinking more about some thirdparty middleware (outside TG's
> code-base, hence we can't tell it to doom the transaction) which used
> the response status to signal an error and doesn't want to deal with
> transactions because it doens't need them.  This imaginary middleware
> needs to be stacked below TM however since I want TM to wrap several
> apps (eg: 'forked' with a URLMap) but only one of these needs the
> imaginary middleware in its branch since if it were above TM it would
> affect both of them.
> It has just occurred to me that we could still wrap this troublesome
> middleware with yet another middleware that turned this HTTP error back
> into an exception (or simply doom the transaction) but I think that the
> hook you proposed would make things simpler since TG2 could configure TM
> with this behavior by default so users who want to cherry pick their
> middleware pieces don't get bitten by, for example, 403 responses
> silently committing whatever objects have been created until the auth.
> framework determined the user didn't have the credentials.
> As I've said, this bug happened to me and I found it pretty obscure
> since all I got was a report by a co-worker who was testing the app that
> said: "I tried to delete a mailbox I wasn't supposed to delete and the
> app told me I couldn't, but then I looked back and it wasn't there!"
>>>> If that's not convenient, how about a way to define a "commit veto"
>>>> hook?
>>>> Then
>>>> you could do whatever you liked with the environ, status, or response
>>>> headers to
>>>> determine that you shouldn't commit. An implementation might be:
>>>> def commit_veto(environ, status, response_headers):
>>>>      if not status.startswith('2'):
>>>>          return True
>>>> It would be called by the middleware just before it's about to
>>>> commit.  If
>>>> the
>>>> commit veto hook return True, it would mean "don't commit".  Tthe
>>>> middleware
>>>> would capture the status and header output and do:
>>>> if transaction.isDoomed() or self.commit_veto(environ, status,
>>>> headers):
>>>>      self.abort()
>>>> The commit_veto function would be passed as an argument to the
>>>> repoze.tm
>>>> middleware constructor, so you'd create a TM via:
>>>> txn_managed_app = repoze.tm2.TM(app, commit_veto)
>>> This would be prefect :)
>> OK, I'll try to add it in some form shortly (or you can if you like
>> too, I'd be happy to accept a patch or give commit access).  In the
>> meantime, you might just try to work around the issue with a doom.
> I've attached a patch implementing the commit_veto() hook with a test. I
> had to modify some of the tests to send a dummy start_response callable
> instead of the dummy None since a closure inside __call__ needs to call
> it (like a real app) after grabbing the status and headers but these
> changes shouldn't affect what was being tested, I hope. Commit access
> would be handy in case I missed something, or perhaps you prefer me to
> bug you with a couple more patches before that... :)
> Thanks!
> Alberto

Repoze-dev mailing list

Reply via email to