Code review comment for lp://staging/~jtv/launchpad/db-bug-752279

Revision history for this message
William Grant (wgrant) wrote :

Looks like a great simplification. A few nitpicks:

45 +def get_backup_dists(archive_config):
46 + """Return the path of an archive's backup dists directory."""
47 + return os.path.join(archive_config.archiveroot + "-distscopy", "dists")

Is archiveroot guaranteed to not have a trailing separator?

95 + The publishable files are kept in the filesystem. Most of the work
96 + is done in a working "dists" directory in each archive's dists copy
97 + root, which then replaces the current "dists" in the archive root.

Isn't that false now? Work is done in archiveroot, not distscopyroot.

331 + For each archive, this switches the dists directory and the
332 + backup dists directory around.

Lies! It rotates them through the three dirs.

409 + def publish(self, security_only=False):
410 + """Do the main publishing work.
[...]

Could the except/recoverWorkingDists/raise be a finally instead? It also seems like rsyncBackupDists might belong in this method, as installDists is there.

513 + """Write a marker file for checking direction movements.

s/direction/directory/?

review: Approve (code*)

« Back to merge proposal