Merge lp:~cjwatson/launchpad/optimise-bin-py into lp:launchpad

[Colin Watson]

Optimise lp_sitecustomize so that bin/py starts up more quickly.

This takes bin/py from 0.76 seconds to 0.43 seconds on my system. (The difference is even more pronounced following https://code.launchpad.net/~cjwatson/launchpad/virtualenv-pip/+merge/331388, where importing pkg_resources is apparently a bit more expensive.)

https://code.launchpad.net/~cjwatson/lazr.config/faster-version/+merge/331833 and https://code.launchpad.net/~cjwatson/lazr.delegates/faster-version/+merge/331834 must land and be released first.

[William Grant (wgrant)]

review: Approve (code)

=== removed symlink 'bzrplugins/hg'
=== target was u'../sourcecode/bzr-hg'
=== modified file 'lib/lp/services/config/__init__.py'
--- lib/lp/services/config/__init__.py	2016-10-04 11:19:07 +0000
+++ lib/lp/services/config/__init__.py	2017-10-05 13:43:06 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2016 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2017 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 '''
@@ -22,7 +22,6 @@
 
 from lazr.config import ImplicitTypeSchema
 from lazr.config.interfaces import ConfigErrors
-import pkg_resources
 import ZConfig
 
 from lp.services.osutils import open_for_writing
@@ -241,6 +240,7 @@
 
     def _setZConfig(self):
         """Modify the config, adding automatically generated settings"""
+        import pkg_resources
         schemafile = pkg_resources.resource_filename(
             'zope.app.server', 'schema.xml')
         schema = ZConfig.loadSchema(schemafile)

=== modified file 'lib/lp/services/config/doc/canonical-config.txt'
--- lib/lp/services/config/doc/canonical-config.txt	2015-10-01 22:36:23 +0000
+++ lib/lp/services/config/doc/canonical-config.txt	2017-10-05 13:43:06 +0000
@@ -59,7 +59,7 @@
     '.../launchpad-lazr.conf'
 
     >>> config.extends.filename
-    '.../launchpad-lazr.conf'
+    u'.../launchpad-lazr.conf'
 
 LaunchpadConfig provides __contains__ and __getitem__ to check and
 access lazr.config sections and keys.
@@ -160,7 +160,7 @@
     '.../configs/testrunner/test-process-lazr.conf'
 
     >>> test_config.extends.filename
-    '.../configs/testrunner/launchpad-lazr.conf'
+    u'.../configs/testrunner/launchpad-lazr.conf'
 
     >>> test_config.answertracker.days_before_expiration
     300
@@ -180,7 +180,7 @@
     '.../configs/testrunner/launchpad-lazr.conf'
 
     >>> test_config.extends.filename
-    '.../configs/development/launchpad-lazr.conf'
+    u'.../configs/development/launchpad-lazr.conf'
 
     >>> test_config.answertracker.days_before_expiration
     15
@@ -220,7 +220,7 @@
 #    >>> config.filename
 #    '.../configs/mailman-itests/launchpad-lazr.conf'
 #    >>> config.extends.filename
-#    '.../configs/development/launchpad-lazr.conf'
+#    u'.../configs/development/launchpad-lazr.conf'
 #    >>> config.database.dbname
 #    'launchpad_dev'
 

=== modified file 'lib/lp/services/log/logger.py'
--- lib/lp/services/log/logger.py	2014-08-29 04:29:16 +0000
+++ lib/lp/services/log/logger.py	2017-10-05 13:43:06 +0000
@@ -1,4 +1,4 @@
-# Copyright 2010-2011 Canonical Ltd.  This software is licensed under the
+# Copyright 2010-2017 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Loggers."""
@@ -17,11 +17,6 @@
 import sys
 import traceback
 
-from testtools.content import (
-    Content,
-    UTF8_TEXT,
-    )
-
 from lp.services.log import loglevels
 
 
@@ -224,5 +219,10 @@
         Use with `testtools.TestCase.addDetail`, `fixtures.Fixture.addDetail`,
         and anything else that understands details.
         """
+        # Only import these here to avoid importing testtools outside tests.
+        from testtools.content import (
+            Content,
+            UTF8_TEXT,
+            )
         get_bytes = lambda: [self.getLogBuffer().encode("utf-8")]
         return Content(UTF8_TEXT, get_bytes)

=== modified file 'lib/lp/services/webapp/initialization.py'
--- lib/lp/services/webapp/initialization.py	2010-08-20 20:31:18 +0000
+++ lib/lp/services/webapp/initialization.py	2017-10-05 13:43:06 +0000
@@ -1,4 +1,4 @@
-# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# Copyright 2010-2017 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Initializes the application after ZCML has been processed."""
@@ -8,15 +8,19 @@
     getSiteManager,
     )
 from zope.interface import (
+    alsoProvides,
     implementer,
     Interface,
     )
 from zope.processlifetime import IDatabaseOpened
+import zope.publisher.browser
 from zope.publisher.interfaces import IRequest
 from zope.publisher.interfaces.browser import IBrowserRequest
 from zope.publisher.interfaces.http import IHTTPRequest
 from zope.traversing.interfaces import ITraversable
 
+from lp.services.webapp.interfaces import IUnloggedException
+
 
 @implementer(Interface)
 def adapter_mask(*args):
@@ -37,6 +41,7 @@
     Python first starts.
     """
     fix_up_namespace_traversers()
+    customize_get_converter()
 
 
 def fix_up_namespace_traversers():
@@ -61,3 +66,37 @@
                 sm.registerAdapter(
                     adapter_mask,
                     required=(Interface, request_iface), name=name, info=info)
+
+
+def customize_get_converter(zope_publisher_browser=zope.publisher.browser):
+    """URL parameter conversion errors shouldn't generate an OOPS report.
+
+    This injects (monkey patches) our wrapper around get_converter so improper
+    use of parameter type converters (like http://...?foo=bar:int) won't
+    generate OOPS reports.
+
+    This is done in a function rather than in ZCML because zope.publisher
+    doesn't provide fine enough control of this any other way.
+    """
+    original_get_converter = zope_publisher_browser.get_converter
+
+    def get_converter(*args, **kws):
+        """Get a type converter but turn off OOPS reporting if it fails."""
+        converter = original_get_converter(*args, **kws)
+
+        def wrapped_converter(v):
+            try:
+                return converter(v)
+            except ValueError as e:
+                # Mark the exception as not being OOPS-worthy.
+                alsoProvides(e, IUnloggedException)
+                raise
+
+        # The converter can be None, in which case wrapping it makes no sense,
+        # otherwise it is a function which we wrap.
+        if converter is None:
+            return None
+        else:
+            return wrapped_converter
+
+    zope_publisher_browser.get_converter = get_converter

=== modified file 'lib/lp/services/webapp/tests/test_errorlog.py'
--- lib/lp/services/webapp/tests/test_errorlog.py	2017-09-02 13:29:14 +0000
+++ lib/lp/services/webapp/tests/test_errorlog.py	2017-10-05 13:43:06 +0000
@@ -13,7 +13,6 @@
 from fixtures import TempDir
 from lazr.batchnavigator.interfaces import InvalidBatchSizeError
 from lazr.restful.declarations import error_status
-from lp_sitecustomize import customize_get_converter
 import oops_amqp
 import pytz
 import testtools
@@ -611,77 +610,6 @@
         self.assertTrue(report['ignore'])
 
 
-class TestWrappedParameterConverter(testtools.TestCase):
-    """Make sure URL parameter type conversions don't generate OOPS reports"""
-
-    def test_return_value_untouched(self):
-        # When a converter succeeds, its return value is passed through the
-        # wrapper untouched.
-
-        class FauxZopePublisherBrowserModule:
-            def get_converter(self, type_):
-                def the_converter(value):
-                    return 'converted %r to %s' % (value, type_)
-                return the_converter
-
-        module = FauxZopePublisherBrowserModule()
-        customize_get_converter(module)
-        converter = module.get_converter('int')
-        self.assertEqual("converted '42' to int", converter('42'))
-
-    def test_value_errors_marked(self):
-        # When a ValueError is raised by the wrapped converter, the exception
-        # is marked with IUnloggedException so the OOPS machinery knows that a
-        # report should not be logged.
-
-        class FauxZopePublisherBrowserModule:
-            def get_converter(self, type_):
-                def the_converter(value):
-                    raise ValueError
-                return the_converter
-
-        module = FauxZopePublisherBrowserModule()
-        customize_get_converter(module)
-        converter = module.get_converter('int')
-        try:
-            converter(42)
-        except ValueError as e:
-            self.assertTrue(IUnloggedException.providedBy(e))
-
-    def test_other_errors_not_marked(self):
-        # When an exception other than ValueError is raised by the wrapped
-        # converter, the exception is not marked with IUnloggedException an
-        # OOPS report will be created.
-
-        class FauxZopePublisherBrowserModule:
-            def get_converter(self, type_):
-                def the_converter(value):
-                    raise RuntimeError
-                return the_converter
-
-        module = FauxZopePublisherBrowserModule()
-        customize_get_converter(module)
-        converter = module.get_converter('int')
-        try:
-            converter(42)
-        except RuntimeError as e:
-            self.assertFalse(IUnloggedException.providedBy(e))
-
-    def test_none_is_not_wrapped(self):
-        # The get_converter function that we're wrapping can return None, in
-        # that case there's no function for us to wrap and we just return None
-        # as well.
-
-        class FauxZopePublisherBrowserModule:
-            def get_converter(self, type_):
-                return None
-
-        module = FauxZopePublisherBrowserModule()
-        customize_get_converter(module)
-        converter = module.get_converter('int')
-        self.assertTrue(converter is None)
-
-
 class TestHooks(testtools.TestCase):
 
     def test_attach_http_nonbasicvalue(self):

=== modified file 'lib/lp/services/webapp/tests/test_initialization.py'
--- lib/lp/services/webapp/tests/test_initialization.py	2012-01-01 02:58:52 +0000
+++ lib/lp/services/webapp/tests/test_initialization.py	2017-10-05 13:43:06 +0000
@@ -1,4 +1,4 @@
-# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# Copyright 2010-2017 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Tests post-zcml application initialization.
@@ -7,10 +7,12 @@
 
 from zope.component import getSiteManager
 from zope.interface import Interface
+from zope.publisher import browser
 from zope.publisher.interfaces.browser import IBrowserRequest
 from zope.traversing.interfaces import ITraversable
 
 from lp.services.webapp.errorlog import OopsNamespace
+from lp.services.webapp.interfaces import IUnloggedException
 from lp.services.webapp.servers import LaunchpadTestRequest
 from lp.testing import TestCase
 from lp.testing.layers import FunctionalLayer
@@ -55,3 +57,46 @@
                 pass
             else:
                 self.assertNotEqual(factory, not_the_namespace_factory)
+
+
+class TestWrappedParameterConverter(TestCase):
+    """Make sure URL parameter type conversions don't generate OOPS reports"""
+
+    layer = FunctionalLayer
+
+    def test_return_value_untouched(self):
+        # When a converter succeeds, its return value is passed through the
+        # wrapper untouched.
+        converter = browser.get_converter('int')
+        self.assertEqual(42, converter('42'))
+
+    def test_value_errors_marked(self):
+        # When a ValueError is raised by the wrapped converter, the exception
+        # is marked with IUnloggedException so the OOPS machinery knows that a
+        # report should not be logged.
+        converter = browser.get_converter('int')
+        try:
+            converter('not an int')
+        except ValueError as e:
+            self.assertTrue(IUnloggedException.providedBy(e))
+
+    def test_other_errors_not_marked(self):
+        # When an exception other than ValueError is raised by the wrapped
+        # converter, the exception is not marked with IUnloggedException an
+        # OOPS report will be created.
+        class BadString:
+            def __str__(self):
+                raise RuntimeError
+
+        converter = browser.get_converter('string')
+        try:
+            converter(BadString())
+        except RuntimeError as e:
+            self.assertFalse(IUnloggedException.providedBy(e))
+
+    def test_none_is_not_wrapped(self):
+        # The get_converter function that we're wrapping can return None, in
+        # that case there's no function for us to wrap and we just return None
+        # as well.
+        converter = browser.get_converter('unregistered')
+        self.assertIsNone(converter)

=== modified file 'lib/lp_sitecustomize.py'
--- lib/lp_sitecustomize.py	2014-08-29 05:52:23 +0000
+++ lib/lp_sitecustomize.py	2017-10-05 13:43:06 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2017 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 # This file is imported by parts/scripts/sitecustomize.py, as set up in our
@@ -10,20 +10,16 @@
 import os
 import warnings
 
-from swiftclient import client as swiftclient
 from twisted.internet.defer import (
     Deferred,
     DeferredList,
     )
-from zope.interface import alsoProvides
-import zope.publisher.browser
 from zope.security import checker
 
 from lp.services.log import loglevels
 from lp.services.log.logger import LaunchpadLogger
 from lp.services.log.mappingfilter import MappingFilter
 from lp.services.mime import customizeMimetypes
-from lp.services.webapp.interfaces import IUnloggedException
 
 
 def add_custom_loglevels():
@@ -95,7 +91,8 @@
     only does swiftclient then emit lots of noise, but it also turns
     keystoneclient debugging on.
     """
-    swiftclient.logger.setLevel(logging.INFO)
+    swiftclient_logger = logging.getLogger('swiftclient')
+    swiftclient_logger.setLevel(logging.INFO)
 
 
 def silence_zcml_logger():
@@ -175,37 +172,6 @@
     silence_swiftclient_logger()
 
 
-def customize_get_converter(zope_publisher_browser=zope.publisher.browser):
-    """URL parameter conversion errors shouldn't generate an OOPS report.
-
-    This injects (monkey patches) our wrapper around get_converter so improper
-    use of parameter type converters (like http://...?foo=bar:int) won't
-    generate OOPS reports.
-    """
-    original_get_converter = zope_publisher_browser.get_converter
-
-    def get_converter(*args, **kws):
-        """Get a type converter but turn off OOPS reporting if it fails."""
-        converter = original_get_converter(*args, **kws)
-
-        def wrapped_converter(v):
-            try:
-                return converter(v)
-            except ValueError as e:
-                # Mark the exception as not being OOPS-worthy.
-                alsoProvides(e, IUnloggedException)
-                raise
-
-        # The converter can be None, in which case wrapping it makes no sense,
-        # otherwise it is a function which we wrap.
-        if converter is None:
-            return None
-        else:
-            return wrapped_converter
-
-    zope_publisher_browser.get_converter = get_converter
-
-
 def main(instance_name):
     # This is called by our custom buildout-generated sitecustomize.py
     # in parts/scripts/sitecustomize.py. The instance name is sent to
@@ -231,4 +197,3 @@
     # through actually using itertools.groupby.
     grouper = type(list(itertools.groupby([0]))[0][1])
     checker.BasicTypes[grouper] = checker._iteratorChecker
-    customize_get_converter()

=== modified file 'versions.cfg'
--- versions.cfg	2017-09-25 12:24:39 +0000
+++ versions.cfg	2017-10-05 13:43:06 +0000
@@ -53,8 +53,8 @@
 launchpadlib = 1.10.5
 lazr.authentication = 0.1.1
 lazr.batchnavigator = 1.2.11
-lazr.config = 1.1.3
-lazr.delegates = 2.0.3
+lazr.config = 2.2.1
+lazr.delegates = 2.0.4
 lazr.enum = 1.1.3
 lazr.jobrunner = 0.13
 lazr.lifecycle = 1.1
@@ -114,7 +114,7 @@
 python-subunit = 0.0.8beta
 python-swiftclient = 1.5.0
 PyYAML = 3.10
-pytz = 2016.4
+pytz = 2017.2
 rabbitfixture = 0.3.6
 requests = 2.7.0
 requests-toolbelt = 0.6.2