Merge lp:~jtv/launchpad/bug-723168 into lp:launchpad
Status: | Merged | ||||
---|---|---|---|---|---|
Approved by: | Jeroen T. Vermeulen | ||||
Approved revision: | no longer in the source branch. | ||||
Merged at revision: | 12439 | ||||
Proposed branch: | lp:~jtv/launchpad/bug-723168 | ||||
Merge into: | lp:launchpad | ||||
Diff against target: |
62 lines (+34/-2) 3 files modified
database/schema/security.cfg (+1/-1) lib/lp/translations/model/potemplate.py (+1/-1) lib/lp/translations/scripts/tests/test_verify_pofile_stats.py (+32/-0) |
||||
To merge this branch: | bzr merge lp:~jtv/launchpad/bug-723168 | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Leonard Richardson (community) | Approve | ||
Review via email: mp+50808@code.launchpad.net |
Commit message
[r=leonardr][bug=723168] Fix database permissions error (and some of the slowness) in POFile stats verifier.
Description of the change
= Summary =
The POFile statistics verifier script was failing in production because it was selecting from the ProductSeries table, and it had no permission to.
Thing is, there's no reason why it should. The only reason it's accessing Product is because POTemplate.
== Proposed fix ==
Check the productseriesID foreign-key attribute for Noneness, rather than the productseries reference attribute. Presto: no need for further queries, and a faster script to boot.
Also, I noticed that whereas POFile.
== Pre-implementation notes ==
I did not have a proper pre-imp for this, though I worked on it with a LOSA and informed the Translations veterans. It's just too basic.
== Implementation details ==
I added a test that reproduces the problem. This is a style of testing I've gotten into recently: unit-test scripts in lightweight layers with no concern for permissions, then have a separate test case that just exercises database permissions. With the database commits (and also the layer setup, if you're running isolated tests) it's just a waste of time to run everything in a heavyweight layer.
Note that the test exercises POFile.
== Tests ==
{{{
./bin/test -vvc lp.translations
}}}
== Demo and Q/A ==
Run cronsctipts/
= Launchpad lint =
Ignore the security.cfg lint. We really shouldn't be checking line lengths in that file at all, but changes to the file are rare enough that nobody bothers to fix this.
Checking for conflicts and issues in changed files.
Linting changed files:
database/
lib/lp/
lib/lp/
./database/
726: Line exceeds 78 characters.
727: Line exceeds 78 characters.
728: Line exceeds 78 characters.
754: Line exceeds 78 characters.
758: Line exceeds 78 characters.
813: Line exceeds 78 characters.
826: Line exceeds 78 characters.
827: Line exceeds 78 characters.
843: Line exceeds 78 characters.
844: Line exceeds 78 characters.
845: Line exceeds 78 characters.
846: Line exceeds 78 characters.
847: Line exceeds 78 characters.
899: Line exceeds 78 characters.
900: Line exceeds 78 characters.
901: Line exceeds 78 characters.
931: Line exceeds 78 characters.
1012: Line exceeds 78 characters.
1022: Line exceeds 78 characters.
1023: Line exceeds 78 characters.