Code review comment for lp:~jtv/launchpad/bug-536769

Hi Jelmer,

Thanks for the review.

> 45 + def _becomeBuilddMaster(self):
> 46 + """Log into the database as the buildd master."""
> 47 + transaction.commit()
> ^^^ Is the transaction.commit() really necessary here, would a Store.flush not
> work ?

Good question to ask! In this case, yes, we're switching database identities. And you can't do that in the same transaction, so flushing wouldn't make the most recent data changes visible.

This is a pattern we come across sometimes in our test: you want to test what you do on the database under realistic permissions, but those permissions aren't enough to set up your test data. The only choice is to commit and switch identities. And not do it too often, of course. :-)

Jeroen