launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/obsolete-known-broken-scripts into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #403551 in Launchpad itself: "Nonconforming scripts"
  https://bugs.launchpad.net/launchpad/+bug/403551

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/obsolete-known-broken-scripts/+merge/112365

== Summary ==

lp.services.scripts.tests.KNOWN_BROKEN is old and crufty.

== Proposed fix ==

Some of the entries in lp.services.scripts.tests.KNOWN_BROKEN were for scripts that no longer exist any more, some were for cases that don't need to be listed explicitly (missing .py extension), and some were legitimate listings for missing --help output.  I added --help output to the three scripts that lacked it, and removed the facility to have broken scripts; there should never be a need to add these in the future, since including at least minimal --help output is trivial.

== Tests ==

bin/test -vvct test_all_scripts

== Lint ==

Some _pythonpath false positives and some general untidiness in start-loggerhead.  This was pre-existing and doesn't seem immediately worth cleaning up here.

./scripts/get-stacked-on-branches.py
      28: '_pythonpath' imported but unused
./scripts/start-loggerhead.py
       6: '_pythonpath' imported but unused
      22: 'lp' imported but unused
      64: E501 line too long (88 characters)
      89: E301 expected 1 blank line, found 0
     137: E302 expected 2 blank lines, found 0
     143: E301 expected 1 blank line, found 0
     146: E301 expected 1 blank line, found 0
     157: E302 expected 2 blank lines, found 1
./scripts/stop-loggerhead.py
       6: '_pythonpath' imported but unused
-- 
https://code.launchpad.net/~cjwatson/launchpad/obsolete-known-broken-scripts/+merge/112365
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/obsolete-known-broken-scripts into lp:launchpad.

=== modified file 'lib/lp/services/scripts/tests/__init__.py'
--- lib/lp/services/scripts/tests/__init__.py	2012-02-28 05:36:01 +0000
+++ lib/lp/services/scripts/tests/__init__.py	2012-06-27 14:04:27 +0000
@@ -23,30 +23,6 @@
     ]
 
 
-KNOWN_BROKEN = [
-    # Needs mysqldb module
-    'scripts/migrate-bugzilla-initialcontacts.py',
-    'scripts/rosetta/gettext_check_messages.py',
-    # sqlobject.DatbaseIndex ?
-    'scripts/linkreport.py',
-    # Python executable without '.py' extension.
-    'scripts/list-team-members',
-    'scripts/queue',
-    # Bad script, no help.
-    'scripts/librarian-report.py',
-    'scripts/get-stacked-on-branches.py',
-    'scripts/start-loggerhead.py',
-    'scripts/stop-loggerhead.py',
-    ]
-
-
-def is_broken(script_path):
-    for broken_path in KNOWN_BROKEN:
-        if script_path.endswith(broken_path):
-            return True
-    return False
-
-
 def find_lp_scripts():
     """Find all scripts/ and cronscripts/ files in the current tree.
 
@@ -60,8 +36,7 @@
             for filename in filenames:
                 script_path = os.path.join(path, filename)
                 if (filename.startswith('_') or
-                    not filename.endswith('.py') or
-                    is_broken(script_path)):
+                    not filename.endswith('.py')):
                     continue
                 scripts.append(script_path)
     return sorted(scripts)

=== modified file 'scripts/get-stacked-on-branches.py'
--- scripts/get-stacked-on-branches.py	2012-01-01 03:13:08 +0000
+++ scripts/get-stacked-on-branches.py	2012-06-27 14:04:27 +0000
@@ -1,6 +1,6 @@
 #!/usr/bin/python -S
 #
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 # pylint: disable-msg=W0403
@@ -27,6 +27,8 @@
 
 import _pythonpath
 
+from optparse import OptionParser
+
 from storm.locals import Not
 from zope.component import getUtility
 
@@ -51,6 +53,10 @@
 
     See the module docstring for more information.
     """
+    parser = OptionParser(
+        description="List the stacked branches in Launchpad.")
+    parser.parse_args()
+
     execute_zcml_for_scripts()
     for db_branch in get_stacked_branches():
         stacked_on = db_branch.stacked_on

=== modified file 'scripts/start-loggerhead.py'
--- scripts/start-loggerhead.py	2012-01-01 03:13:08 +0000
+++ scripts/start-loggerhead.py	2012-06-27 14:04:27 +0000
@@ -1,11 +1,12 @@
 #!/usr/bin/python -S
 #
-# Copyright 2009, 2010 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 import _pythonpath
 
 import logging
+from optparse import OptionParser
 import os
 import sys
 import time
@@ -90,16 +91,16 @@
         sys.stderr = S()
 
 
-
-foreground = False
-if len(sys.argv) > 1:
-    if sys.argv[1] == '-f':
-        foreground = True
+parser = OptionParser(description="Start loggerhead.")
+parser.add_option(
+    "-f", "--foreground", default=False, action="store_true",
+    help="Run loggerhead in the foreground.")
+options, _ = parser.parse_args()
 
 home = os.path.realpath(os.path.dirname(__file__))
 pidfile = os.path.join(home, 'loggerhead.pid')
 
-if not foreground:
+if not options.foreground:
     sys.stderr.write('\n')
     sys.stderr.write('Launching loggerhead into the background.\n')
     sys.stderr.write('PID file: %s\n' % (pidfile,))
@@ -108,7 +109,7 @@
     from loggerhead.daemon import daemonize
     daemonize(pidfile, home)
 
-setup_logging(home, foreground=foreground)
+setup_logging(home, foreground=options.foreground)
 
 log = logging.getLogger('loggerhead')
 log.info('Starting up...')

=== modified file 'scripts/stop-loggerhead.py'
--- scripts/stop-loggerhead.py	2012-01-01 03:13:08 +0000
+++ scripts/stop-loggerhead.py	2012-06-27 14:04:27 +0000
@@ -1,14 +1,18 @@
 #!/usr/bin/python -S
 #
-# Copyright 2009, 2010 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 import _pythonpath
 
+from optparse import OptionParser
 import os
 import sys
 
 
+parser = OptionParser(description="Stop loggerhead.")
+parser.parse_args()
+
 home = os.path.realpath(os.path.dirname(__file__))
 pidfile = os.path.join(home, 'loggerhead.pid')