launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:lazy-ZConfig into launchpad:master.

Commit message:
Make LaunchpadConfig load ZConfig lazily

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/387484

Loading ZConfig (launchpad.conf) is relatively heavyweight compared to the rest of Launchpad's configuration system: it involves loading schema resources from several packages and running an XML parser on them.  LaunchpadConfig only needs it for the "devmode" and "servers" settings, which are used quite infrequently.  Load it lazily to save time.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:lazy-ZConfig into launchpad:master.

diff --git a/lib/lp/services/config/__init__.py b/lib/lp/services/config/__init__.py
index ebe9ebf..2a1ae31 100644
--- a/lib/lp/services/config/__init__.py
+++ b/lib/lp/services/config/__init__.py
@@ -112,8 +112,8 @@ class LaunchpadConfig:
         :param process_name: the process configuration name to use. Defaults
             to the basename of sys.argv[0] without any extension, or None if
             sys.argv is not available.
-       """
-        self._config = None
+        """
+        self._invalidateConfig()
         if instance_name is None:
             instance_name = find_instance_name()
 
@@ -162,6 +162,8 @@ class LaunchpadConfig:
     def _invalidateConfig(self):
         """Invalidate the config, causing the config to be regenerated."""
         self._config = None
+        self._devmode = None
+        self._servers = None
 
     def reloadConfig(self):
         """Reload the config."""
@@ -224,7 +226,6 @@ class LaunchpadConfig:
         except ConfigErrors as error:
             message = '\n'.join([str(e) for e in error.errors])
             raise ConfigErrors(message)
-        self._setZConfig()
 
     def _loadConfigOverlays(self, config_file):
         """Apply config overlays from the launchpad.config_overlay_dir."""
@@ -243,22 +244,35 @@ class LaunchpadConfig:
         """Return the path to the ZConfig file for this instance."""
         return os.path.join(self.config_dir, 'launchpad.conf')
 
-    def _setZConfig(self):
+    def _getZConfig(self):
         """Modify the config, adding automatically generated settings"""
         with resources.path('zope.app.server', 'schema.xml') as schemafile:
             schema = ZConfig.loadSchema(str(schemafile))
         root_options, handlers = ZConfig.loadConfig(
             schema, self.zope_config_file)
+        self._devmode = root_options.devmode
+        self._servers = root_options.servers
 
-        # Devmode from the zope.app.server.main config, copied here for
-        # ease of access.
-        self.devmode = root_options.devmode
+    @property
+    def devmode(self):
+        """Devmode from the zope.app.server.main config.
 
-        # The defined servers.
-        self.servers = root_options.servers
+        Copied here for ease of access.
+        """
+        if self._devmode is None:
+            self._getZConfig()
+        return self._devmode
+
+    @devmode.setter
+    def devmode(self, value):
+        self._devmode = value
 
-        # The number of configured threads.
-        self.threads = root_options.threads
+    @property
+    def servers(self):
+        """The defined servers."""
+        if self._servers is None:
+            self._getZConfig()
+        return self._servers
 
     def generate_overrides(self):
         """Ensure correct config.zcml overrides will be called.
diff --git a/lib/lp/services/config/doc/canonical-config.txt b/lib/lp/services/config/doc/canonical-config.txt
index 7bb4575..91f676b 100644
--- a/lib/lp/services/config/doc/canonical-config.txt
+++ b/lib/lp/services/config/doc/canonical-config.txt
@@ -89,9 +89,6 @@ ZConfig.
     >>> len(config.servers)
     4
 
-    >>> config.threads
-    4
-
 
 Working with test configurations
 --------------------------------
diff --git a/lib/lp/services/config/tests/test_config_lookup.py b/lib/lp/services/config/tests/test_config_lookup.py
index cc3bdd5..6c41492 100644
--- a/lib/lp/services/config/tests/test_config_lookup.py
+++ b/lib/lp/services/config/tests/test_config_lookup.py
@@ -125,8 +125,6 @@ class TestInstanceConfigDirLookup(ConfigTestCase):
         config_file.write('[launchpad]\ndefault_batch_size=2323')
         config_file.close()
 
-        # We don't care about ZConfig...
-        cfg._setZConfig = lambda: None
         self.assertEqual(2323, cfg.launchpad.default_batch_size)