launchpad-reviewers team mailing list archive

["Jeroen T. Vermeulen" <jtv@xxxxxxxxxxxxx>]

Jeroen T. Vermeulen has proposed merging lp:~jtv/maas/bug-1065055 into lp:maas.

Commit message:
Duplicate import_settings between packages for now; cluster installation (specifically celeryconfig_cluster.py) can't get at the helper's original definition in the maas package which we had wanted to share.

Bug 1065456 reminds us that we need to resolve this code duplication.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1065055 in maas (Ubuntu): "celeryconfig_cluster.py imports utility method from maas (import_settings)"
  https://bugs.launchpad.net/ubuntu/+source/maas/+bug/1065055

For more details, see:
https://code.launchpad.net/~jtv/maas/bug-1065055/+merge/129153

As per Raphael.  This resolves an installation problem for the cluster controller.  The regular celeryconfig.py doesn't have this problem, since it only gets imported from an upstart job that does have maas in its python path.


Jeroen
-- 
https://code.launchpad.net/~jtv/maas/bug-1065055/+merge/129153
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/maas/bug-1065055 into lp:maas.

=== modified file 'etc/celeryconfig_cluster.py'
--- etc/celeryconfig_cluster.py	2012-09-29 17:25:17 +0000
+++ etc/celeryconfig_cluster.py	2012-10-11 10:57:34 +0000
@@ -18,7 +18,7 @@
 from datetime import timedelta
 
 import celeryconfig_common
-from maas import import_settings
+from provisioningserver.utils import import_settings
 
 # Cluster UUID.  Will be overridden by the customized setting in the
 # local MAAS Celery config.

=== modified file 'src/maas/__init__.py'
--- src/maas/__init__.py	2012-04-16 10:00:51 +0000
+++ src/maas/__init__.py	2012-10-11 10:57:34 +0000
@@ -20,6 +20,10 @@
 
 def find_settings(whence):
     """Return settings from `whence`, which is assumed to be a module."""
+    # XXX 2012-10-11 JeroenVermeulen, bug=1065456: We thought this would be
+    # a good shared location for this helper, but we can't get at it during
+    # cluster installation.  So it's currently duplicated.  Put it in a
+    # properly shared location.
     return {
         name: value
         for name, value in vars(whence).items()
@@ -29,6 +33,10 @@
 
 def import_settings(whence):
     """Import settings from `whence` into the caller's global scope."""
+    # XXX 2012-10-11 JeroenVermeulen, bug=1065456: We thought this would be
+    # a good shared location for this helper, but we can't get at it during
+    # cluster installation.  So it's currently duplicated.  Put it in a
+    # properly shared location.
     source = find_settings(whence)
     target = sys._getframe(1).f_globals
     target.update(source)

=== modified file 'src/provisioningserver/utils.py'
--- src/provisioningserver/utils.py	2012-10-03 09:10:48 +0000
+++ src/provisioningserver/utils.py	2012-10-11 10:57:34 +0000
@@ -14,6 +14,7 @@
     "ActionScript",
     "atomic_write",
     "deferred",
+    "import_settings",
     "incremental_write",
     "MainScript",
     "parse_key_value_file",
@@ -44,6 +45,26 @@
 from twisted.internet.defer import maybeDeferred
 
 
+def find_settings(whence):
+    """Return settings from `whence`, which is assumed to be a module."""
+    # XXX 2012-10-11 JeroenVermeulen, bug=1065456: Put this in a shared
+    # location.  It's currently duplicated from elsewhere.
+    return {
+        name: value
+        for name, value in vars(whence).items()
+        if not name.startswith("_")
+        }
+
+
+def import_settings(whence):
+    """Import settings from `whence` into the caller's global scope."""
+    # XXX 2012-10-11 JeroenVermeulen, bug=1065456: Put this in a shared
+    # location.  It's currently duplicated from elsewhere.
+    source = find_settings(whence)
+    target = sys._getframe(1).f_globals
+    target.update(source)
+
+
 def deferred(func):
     """Decorates a function to ensure that it always returns a `Deferred`.