launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #21903
[Merge] lp:~cjwatson/launchpad/optimise-bin-py into lp:launchpad
Colin Watson has proposed merging lp:~cjwatson/launchpad/optimise-bin-py into lp:launchpad.
Commit message:
Optimise lp_sitecustomize so that bin/py starts up more quickly.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/optimise-bin-py/+merge/331863
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.
--
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/optimise-bin-py into lp:launchpad.
=== 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
Follow ups
