Code review comment for lp:~danilo/launchpad/bug-408718

Hi Henning,

Thanks for the review. For documentation purposes, I am writing up what
we've discussed on IRC as well.

У пон, 14. 09 2009. у 10:24 +0000, Henning Eggers пише:
> > + except TimeoutError:
> > + # Restart the transaction and return empty SelectResults.
> > + transaction.abort()
> > + transaction.begin()
> > + cur.execute("SELECT 1 WHERE 1=0")
>
> As you mentioned on IRC, you should reset the statement_timeout back to
> what it was (if that is not done by abort() ?). Please extend the test
> to check that, too.

I've decided not to (and I don't know if transaction.abort() resets
statement_timeout value): this script run will finish in at most 10
minutes, so having a safeguard while it runs is not too bad (considering
how we just almost killed the DB server).

> Also, could you consider using EmptyResultSet instead of such useless
> queries, please? ;)

As discussed, this is actually using SQLObject, and since I am going to
ask for a CP, I am not converting this query to Storm. With
EmptyResultSet the following would not be possible:

> > rows = cur.fetchall()

And I'd have to worry a bit more about breaking something else.

Cheers,
Danilo