launchpad-reviewers team mailing list archive

[Martin Packman <martin.packman@xxxxxxxxxxxxx>]

Martin Packman has proposed merging lp:~gz/launchpad/sitecustomwhys_bzr_plugin_imports into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~gz/launchpad/sitecustomwhys_bzr_plugin_imports/+merge/121896

Launchpad does crazy things in sitecustomize to appease the zope security devils, including importing large chunks of bzr and several plugins. It also tries to go around silencing as many logs as possible. Amusingly though, it does this after importing everythng, so the warnings bzr logs about plugins being out of date still result in a "No handlers..." message.

When trying to update the version of bzr launchpad uses to 2.5.1 but missing upgrading bzr-loom at the same time, this resulted in a bunch of test failures along the lines of:

    File "lib/lp/services/scripts/tests/test_logger.txt", line 54, in test_logger.txt
    Failed example:
        test("--quiet", "-q")
    Differences (ndiff with -expected +actual):
        + No handlers could be found for logger "bzr"
          ERROR   This is an error

This is not useful. We get test failures for loom anyway, so it's reasonable to just fix the log silencing code to come before the imports. This branch does that, by defering shifting the log bits before the zope security bits, and doing the bzrlib imports at function rather than module level.

I'd like to nuke all this intsead, but that would mean understanding why the hell it exists in the first place.


= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp_sitecustomize.py

-- 
https://code.launchpad.net/~gz/launchpad/sitecustomwhys_bzr_plugin_imports/+merge/121896
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~gz/launchpad/sitecustomwhys_bzr_plugin_imports into lp:launchpad.

=== modified file 'lib/lp_sitecustomize.py'
--- lib/lp_sitecustomize.py	2012-06-29 08:40:05 +0000
+++ lib/lp_sitecustomize.py	2012-08-29 16:19:22 +0000
@@ -10,7 +10,6 @@
 import os
 import warnings
 
-from bzrlib.branch import Branch
 from twisted.internet.defer import (
     Deferred,
     DeferredList,
@@ -19,12 +18,6 @@
 import zope.publisher.browser
 from zope.security import checker
 
-# Load bzr plugins
-import lp.codehosting
-lp.codehosting
-# Force LoomBranch classes to be listed as subclasses of Branch
-import bzrlib.plugins.loom.branch
-bzrlib.plugins.loom.branch
 from lp.services.log import loglevels
 from lp.services.log.logger import LaunchpadLogger
 from lp.services.log.mappingfilter import MappingFilter
@@ -121,6 +114,17 @@
         dont_wrap_class_and_subclasses(subcls)
 
 
+def dont_wrap_bzr_branch_classes():
+    from bzrlib.branch import Branch
+    # Load bzr plugins
+    import lp.codehosting
+    lp.codehosting
+    # Force LoomBranch classes to be listed as subclasses of Branch
+    import bzrlib.plugins.loom.branch
+    bzrlib.plugins.loom.branch
+    dont_wrap_class_and_subclasses(Branch)
+
+
 def silence_warnings():
     """Silence warnings across the entire Launchpad project."""
     # pycrypto-2.0.1 on Python2.6:
@@ -194,7 +198,9 @@
     os.environ['STORM_CEXTENSIONS'] = '1'
     add_custom_loglevels()
     customizeMimetypes()
-    dont_wrap_class_and_subclasses(Branch)
+    silence_warnings()
+    customize_logger()
+    dont_wrap_bzr_branch_classes()
     checker.BasicTypes.update({defaultdict: checker.NoProxy})
     checker.BasicTypes.update({Deferred: checker.NoProxy})
     checker.BasicTypes.update({DeferredList: checker.NoProxy})
@@ -203,6 +209,4 @@
     # through actually using itertools.groupby.
     grouper = type(list(itertools.groupby([0]))[0][1])
     checker.BasicTypes[grouper] = checker._iteratorChecker
-    silence_warnings()
-    customize_logger()
     customize_get_converter()