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

[Julian Edwards]

[Julian Edwards (julian-edwards)]

= 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

[Gavin Panella (allenap)]

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

=== modified file 'lib/lp/soyuz/scripts/expire_ppa_binaries.py'
--- lib/lp/soyuz/scripts/expire_ppa_binaries.py	2009-10-17 14:06:03 +0000
+++ lib/lp/soyuz/scripts/expire_ppa_binaries.py	2009-12-11 14:39:17 +0000
@@ -48,13 +48,18 @@
             "-n", "--dry-run", action="store_true",
             dest="dryrun", metavar="DRY_RUN", default=False,
             help="If set, no transactions are committed")
+        self.parser.add_option(
+            "-e", "--expire-after", action="store", type="int",
+            dest="num_days", metavar="DAYS", default=15,
+            help=("The number of days after which to expire binaries. "
+                  "Must be specified."))
 
-    def determineExpirables(self):
+    def determineExpirables(self, num_days):
         """Return expirable libraryfilealias IDs."""
         # Avoid circular imports.
         from lp.soyuz.interfaces.archive import ArchivePurpose
 
-        stay_of_execution = '30 days'
+        stay_of_execution = '%d days' % num_days
 
         # The subquery here has to repeat the checks for privacy and
         # blacklisting on *other* publications that are also done in
@@ -105,10 +110,13 @@
 
     def main(self):
         self.logger.info('Starting the PPA binary expiration')
+        num_days = self.options.num_days
+        self.logger.info("Expiring files up to %d days ago" % num_days)
+
         self.store = getUtility(IStoreSelector).get(
             MAIN_STORE, DEFAULT_FLAVOR)
 
-        lfa_ids = self.determineExpirables()
+        lfa_ids = self.determineExpirables(num_days)
         batch_count = 0
         batch_limit = 500
         for id in lfa_ids:

=== modified file 'lib/lp/soyuz/scripts/tests/test_expire_ppa_bins.py'
--- lib/lp/soyuz/scripts/tests/test_expire_ppa_bins.py	2009-08-13 15:12:16 +0000
+++ lib/lp/soyuz/scripts/tests/test_expire_ppa_bins.py	2009-12-11 14:39:17 +0000
@@ -61,6 +61,7 @@
         """Return a PPABinaryExpirer instance."""
         if test_args is None:
             test_args = []
+        test_args.extend(['--expire-after', '30'])
         script = PPABinaryExpirer("test expirer", test_args=test_args)
         script.logger = QuietFakeLogger()
         script.txn = self.layer.txn