The proposal to merge lp:~jtv/launchpad/db-bug-735621 into
lp:launchpad/db-devel has been updated.
Description changed to:
= Summary =
See bug 735621. This converts an untested soyuz bash script to python.
The script in question is cronscripts/publishing/gen-contents/generate-contents.
In addition to converting the script to python and adding tests, this also
breaks the hard-coded tie to Ubuntu and permits the same script to run in
parallel for multiple distributions (though not for the same distribution,
since it would probably cause trouble).
== Implementation details ==
In the diff you'll see a few templates that are used for writing various
configuration scripts that tell the Debian machinery what to do. These were
plain text files with a few replacements made by custom code. I replaced that
with python string interpolation. The parameters are passed as a dict so they
can be accessed by name. There's no other documentation of how this works or
what parameters are passed. With "bzr log" showing me one change in 2007 and
another in 2006 for these files, it's probably more cost-effective for the next
engineer to touch this to figure things out to the extent needed than it is to
write and maintain documentation.
I couldn't place the script in its old location; it won't import _pythonpath if
I put it there. But I gave it a more descriptive name.
== Test ==
I couldn't find any tests for the old script. There's only the new test:
{{{
./bin/test -vvc lp.archivepublisher.tests.test_generate_contents_files
}}}
This doesn't cover executing the script itself. The test is somewhat expensive
as it is, at almost 3 seconds; I don't think another 5+ seconds of ZCML parsing
will pull their weight. There's nothing in there but the usual "import and run
LaunchpadScript" boilerplate. I tried it by hand.
A more interesting hole in test coverage is database privileges. The script
barely accesses the database at all, but it calls LpQueryDistro which does.
The archive publisher's database user (as set in the lazr config) should have
the required privileges. Is this worth an expensive extra test? I'm not sure.
== Demo and Q/A ==
One of the Soyuz gurus will have to verify this. Apparently the script is so
costly that it doesn't even get run on dogfood. I don't expect to see any
changes in performance characteristics.
= Launchpad lint =
The lint is all bogus: files we shouldn't check and one warning we can't avoid.
Checking for conflicts and issues in changed files.
Linting changed files:
lib/canonical/config/schema-lazr.conf
lib/lp/archivepublisher/tests/test_generate_contents_files.py
lib/lp/archivepublisher/scripts/generate_contents_files.py
lib/canonical/launchpad/ftests/script.py
cronscripts/publishing/gen-contents/apt_conf_dist.template
cronscripts/publishing/gen-contents/Contents.top
cronscripts/generate-contents-files.py
cronscripts/publishing/gen-contents/apt_conf_header.template
./lib/canonical/config/schema-lazr.conf
536: Line exceeds 78 characters.
619: Line exceeds 78 characters.
995: Line exceeds 78 characters.
1084: Line exceeds 78 characters.
./cronscripts/publishing/gen-contents/apt_conf_dist.template
4: Line exceeds 78 characters.
5: Line exceeds 78 characters.
./cronscripts/generate-contents-files.py
8: '_pythonpath' imported but unused
For more details, see:
https://code.launchpad.net/~jtv/launchpad/db-bug-735621/+merge/58321
--
https://code.launchpad.net/~jtv/launchpad/db-bug-735621/+merge/58321
Your team Launchpad code reviewers is requested to review the proposed merge of
lp:~jtv/launchpad/db-bug-735621 into lp:launchpad/db-devel.
_______________________________________________
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : [email protected]
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help : https://help.launchpad.net/ListHelp
