launchpad-reviewers team mailing list archive

[Richard Harding <rick.harding@xxxxxxxxxxxxx>]

Review: Approve

Thanks for the update Aaron. 

Only comments are would it make sense for the db user to be an attribute of the job class for the _init_task() call so that there's some context on what that string is.

Would the noop task be more a test helper and be specific to that vs in the list of normal jobs or is it logical that someone might want to use it for actual regular processing at some point?

Finally, the TransactionFreeOperation is a bit confusing as I'd not expect the transaction.commit at the end of something that's not a transaction? Looking at the object, it appears to just be something that says there aren't any current active transactions, so is this just guarding against multiple transactions stomping on each other? Since this is copied from the old code and not pertaining the queues, marking approved but appreciate any education lesson on this front.
-- 
https://code.launchpad.net/~abentley/launchpad/restore-queue-test/+merge/116534
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.