710832 - move balrog transaction objects to a higher level

Status: RESOLVED FIXED

(Release Engineering :: General, defect)

Description

[[e-mail] (:bhearsum)]

While working on bug 693593 I attempted to make a single request completely atomic - that is, if any of the queries performed in a method like auslib.web.views.releases.SingleLocaleView.put, that any changes already made in the same method should be reverted.

In the current design, where transaction objects are created and executed in the low level AUSTable methods, this isn't possible. When these were first added, they were put there on the assumption that transactions could be used to prevent update collisions. With that not being the case anymore, it doesn't make as much sense to keep them there.

Basically, doing this would require an AUSTransaction object to be passed to all of the non-readonly methods of Releases, Rules, Permissions, which would pass them along to AUSTable's insert/update/delete. These methods would add statements to the transaction, but never call .execute(), .commit(), or .rollback(). The web side of things would be on the hook for creating and managing the transaction objects, and therefore have the ability to rollback or commit as required.

Comment 1

[[e-mail] (:bhearsum)]

Attachment: let all queries be part of an uncommitted transaction

This patch is large, but pretty simple. Here's the summary:
* All AUSTable (and subclass) methods that run queries now optionally take an AUSTransaction object.
* When present, select/insert/update/delete will add statements to it, and not commit the transaction. When not present, the old behaviour will be used (create a transaction and commit it before returning)
* Use an error handler to wrap uncaught exceptions in an HTTP 500 error
* Fix some assertEquals to be assertEqual, to be compatible with more Python versions
* Create a base class for admin views to factor out transaction creating/committing/aborting
* Import updates to retry.py

Comment 2

[[e-mail] (:bhearsum)]

Attachment: small improvement to logic

Sorry, I just found some uncommitted changes in my working directory - just a small improvement to the logic in insert/delete/update, and a comment change.

Comment 3

[Rail Aliiev [:rail]]

Comment on attachment 588882 [details] [diff] [review]
small improvement to logic

Looks straight forward to me.

Comment 4

[Chris AtLee [:catlee]]

Comment on attachment 588882 [details] [diff] [review]
small improvement to logic

Review of attachment 588882 [details] [diff] [review]:
-----------------------------------------------------------------

::: auslib/db.py
@@ +331,5 @@
>  
>             @rtype: sqlalchemy.engine.base.ResultProxy
>          """
> +        row = self._returnRowOrRaise(where=where, transaction=trans)
> +        log.debug("balhuteohuntoahuntheonuheo: %s", row)

indeed!

Comment 5

[[e-mail] (:bhearsum)]

Comment on attachment 588882 [details] [diff] [review]
small improvement to logic

https://github.com/mozilla/balrog/commit/c26b4bc70091fa26df8765c7f87adade17dd87bd
https://jenkins.mozilla.org/job/Balrog/51/