Merge lp://staging/~julian-edwards/launchpad/ppa-expire-config-days-bug-476588 into lp://staging/launchpad

Proposed by Julian Edwards
Status: Merged
Approved by: Gavin Panella
Approved revision: not available
Merged at revision: not available
Proposed branch: lp://staging/~julian-edwards/launchpad/ppa-expire-config-days-bug-476588
Merge into: lp://staging/launchpad
Diff against target: 51 lines (+12/-3)
2 files modified
lib/lp/soyuz/scripts/expire_ppa_binaries.py (+11/-3)
lib/lp/soyuz/scripts/tests/test_expire_ppa_bins.py (+1/-0)
To merge this branch: bzr merge lp://staging/~julian-edwards/launchpad/ppa-expire-config-days-bug-476588
Reviewer Review Type Date Requested Status
Gavin Panella (community) Approve
Review via email: mp+15816@code.staging.launchpad.net
To post a comment you must log in.
Revision history for this message
Julian Edwards (julian-edwards) wrote :

= Summary =
ppa-expire-binaries.py now takes a paramater which is the number of days to
keep librarian files around instead of hard-coding it to 30 days.

See bug https://bugs.edge.launchpad.net/bugs/476588

== Proposed fix ==

== Pre-implementation notes ==

== Implementation details ==

== Tests ==
bin/test -cvv test_expire_ppa_bins

I've not tested the two additional error checks on the parameter passed from
the command line, it's rather pointless since this is a script that will be
set up in the crontab and forgotten about.

== Demo and Q/A ==
Dogfood is my friend.

= Launchpad lint =

Checking for conflicts. and issues in doctests and templates.
Running jslint, xmllint, pyflakes, and pylint.
Using normal rules.

Linting changed files:
  lib/lp/soyuz/scripts/tests/test_expire_ppa_bins.py
  lib/lp/soyuz/scripts/expire_ppa_binaries.py

Revision history for this message
Gavin Panella (allenap) wrote :

Hi,

Perhaps it would be better to use a named argument, i.e. add an
--expire-after argument in add_my_options() instead? If specified with
type="int", optparse will also ensure it's an integer.

Being an annoying busy body with a sad and pathetic life, I changed it
to do this:

  http://pastebin.ubuntu.com/338109/

It also uses LaunchpadScriptFailure to signal the error, which means
that the script exits with a status of 1.

Having said that, your branch is fine as it is! Take your pick :)

Gavin.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/soyuz/scripts/expire_ppa_binaries.py'
2--- lib/lp/soyuz/scripts/expire_ppa_binaries.py 2009-10-17 14:06:03 +0000
3+++ lib/lp/soyuz/scripts/expire_ppa_binaries.py 2009-12-11 14:39:17 +0000
4@@ -48,13 +48,18 @@
5 "-n", "--dry-run", action="store_true",
6 dest="dryrun", metavar="DRY_RUN", default=False,
7 help="If set, no transactions are committed")
8+ self.parser.add_option(
9+ "-e", "--expire-after", action="store", type="int",
10+ dest="num_days", metavar="DAYS", default=15,
11+ help=("The number of days after which to expire binaries. "
12+ "Must be specified."))
13
14- def determineExpirables(self):
15+ def determineExpirables(self, num_days):
16 """Return expirable libraryfilealias IDs."""
17 # Avoid circular imports.
18 from lp.soyuz.interfaces.archive import ArchivePurpose
19
20- stay_of_execution = '30 days'
21+ stay_of_execution = '%d days' % num_days
22
23 # The subquery here has to repeat the checks for privacy and
24 # blacklisting on *other* publications that are also done in
25@@ -105,10 +110,13 @@
26
27 def main(self):
28 self.logger.info('Starting the PPA binary expiration')
29+ num_days = self.options.num_days
30+ self.logger.info("Expiring files up to %d days ago" % num_days)
31+
32 self.store = getUtility(IStoreSelector).get(
33 MAIN_STORE, DEFAULT_FLAVOR)
34
35- lfa_ids = self.determineExpirables()
36+ lfa_ids = self.determineExpirables(num_days)
37 batch_count = 0
38 batch_limit = 500
39 for id in lfa_ids:
40
41=== modified file 'lib/lp/soyuz/scripts/tests/test_expire_ppa_bins.py'
42--- lib/lp/soyuz/scripts/tests/test_expire_ppa_bins.py 2009-08-13 15:12:16 +0000
43+++ lib/lp/soyuz/scripts/tests/test_expire_ppa_bins.py 2009-12-11 14:39:17 +0000
44@@ -61,6 +61,7 @@
45 """Return a PPABinaryExpirer instance."""
46 if test_args is None:
47 test_args = []
48+ test_args.extend(['--expire-after', '30'])
49 script = PPABinaryExpirer("test expirer", test_args=test_args)
50 script.logger = QuietFakeLogger()
51 script.txn = self.layer.txn