+def runGerminate(self, override_file, series_name, arch, flavours,
+ structures):
Rightly or wrongly, the “run” in that name led me to expect that this method
would fire up a separate process. Maybe a very short docstring to dispel that
notion?
+germinator =
Getting a better feel for the structure of runGerminate now. Which makes me
wonder:
+# Generate apt-ftparchive extra overrides for Build-Essential
+# fields.
+if ('build-essential' in structure.names and
+flavour == flavours[0]):
+
I think you're right that the file shouldn't be put in place if the
context exits with an exception. If I were writing this without context
managers, then rather than:
with AtomicFile('foo') as handle:
print handle, 'bar'
... I'd probably write:
handle = open('foo.new', 'w')
Ah, I see now why you can't do that: writeOverrides is a closure. It picks up
“structure” and “arch” from the surrounding method at its point of definition.
There's another easy thing to break by accident! The structure should again be
structures[flavours[0]], I guess.
Might it be
On Wed, Dec 07, 2011 at 07:20:29AM -, Jeroen T. Vermeulen wrote:
The part from “series = None” onward seems to be an isolated unit of
work. I think it looks for the first distroseries in development
state, or failing that, the first in frozen state. But the structure
of the code makes
Some very nice tests there! To continue:
=== added file 'lib/lp/archivepublisher/tests/test_generate_extra_overrides.py'
+def file_contents(path):
+Return the contents of the file at path.
+with open(path) as handle:
+return handle.read()
Since you're only reading the file,
[I've not responded to your entire review point-by-point, but I believe
I've addressed all points where I haven't given an explicit response.]
On Wed, Dec 07, 2011 at 09:22:24AM -, Jeroen T. Vermeulen wrote:
+def runGerminate(self, override_file, series_name, arch, flavours,
+
Colin Watson has proposed merging
lp:~cjwatson/launchpad/refactor-cron-germinate into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #899972 in Launchpad itself: cron.germinate is very slow
https://bugs.launchpad.net/launchpad/+bug/899972
The proposal to merge lp:~cjwatson/launchpad/refactor-cron-germinate into
lp:launchpad has been updated.
Commit Message changed to:
Reimplement most of cron.germinate in Python using the new python-germinate
package. This no longer needs to recompute the dependency expansion of common
seeds
Right. I've tracked down all the problems I could find and released germinate
2.1 to fix all of them; none of them were problems with this branch. The
runtime is now down to about 4m50s on mawson; the only differences in output
are (a) in places where it doesn't matter very much and (b)
A note on the problem of ensuring that a PublisherConfig is in place:
factory.makeDistribution creates one for you. Just make sure to pass it a
publish_root_dir, or I think it will use default system config. This should
probably be considered a bug in the factory.
--
It looks to me like you did the right thing in creating a dedicated database
user. We may ditch the per-user privileges, but we do want separate users for
separate scripts.
--
https://code.launchpad.net/~cjwatson/launchpad/refactor-cron-germinate/+merge/84624
Your team Launchpad code reviewers
=== added file 'lib/lp/archivepublisher/scripts/generate_extra_overrides.py'
--- lib/lp/archivepublisher/scripts/generate_extra_overrides.py 1970-01-01
00:00:00 +
+++ lib/lp/archivepublisher/scripts/generate_extra_overrides.py 2011-12-06
15:03:58 +
+class AtomicFile:
+Facilitate
I'm going through the diff sequentially at the moment; the parts I'm ignoring
are the parts that I have nothing to say about. When I get to the end I can
give you a very brief formal review. :)
=== added file 'lib/lp/archivepublisher/scripts/generate_extra_overrides.py'
+def
+def outputPath(self, flavour, series_name, arch, base):
+return os.path.join(
+self.config.germinateroot,
+'%s_%s_%s_%s' % (base, flavour, series_name, arch))
Our dromedary-cased method names normally start with a verb. Read that way,
“outputPath” would
15 matches
Mail list logo
