Merge lp:~frankban/charms/precise/juju-gui/install-yaml into lp:~juju-gui/charms/precise/juju-gui/trunk

[Francesco Banconi]

Fix base Python dependencies.

The install hook imported charmhelpers
before installing python-yaml, and
charmhelpers imports from yaml.

This very old bug has been hidden by the
fact that python-yaml is automatically
installed in ec2/lxc instances, but showed
itself when using the manual provider.

Copy/pasting and integrating the QA from
my last branch, a double check can help
before the release.
QA:
- Bootstrap a Juju environment with --debug.
- Deploy and expose the GUI (make deploy).
- Check juju debug-log (or
  less ~/.juju/local/log/unit-juju-gui-0.log if you
  used lxc): you should see the following message
  at the beginning of the install hook log:
  "Installing base Python dependencies: python-apt,
  python-launchpadlib, python-tempita, python-yaml."
- Wait for the GUI to be ready/started.
- Deploy this bundle: http://pastebin.ubuntu.com/6411548/
- Check everything is ok, xy annotations work (
  the services are vertically aligned), wordpress has
  customized constraints, mysql customized options and
  two units.
- No try to deploy the same bundle again, you
  will see a "services already there" kind of error.

https://codereview.appspot.com/26190043/

[Francesco Banconi (frankban)]

Reviewers: mp+195113_code.launchpad.net,

Message:
Please take a look.

Description:
Fix base Python dependencies.

The install hook imported charmhelpers
before installing python-yaml, and
charmhelpers imports from yaml.

This very old bug has been hidden by the
fact that python-yaml is automatically
installed in ec2/lxc instances, but showed
itself when using the manual provider.

Copy/pasting and integrating the QA from
my last branch, a double check can help
before the release.
QA:
- Bootstrap a Juju environment with --debug.
- Deploy and expose the GUI (make deploy).
- Check juju debug-log (or
   less ~/.juju/local/log/unit-juju-gui-0.log if you
   used lxc): you should see the following message
   at the beginning of the install hook log:
   "Installing base Python dependencies: python-apt,
   python-launchpadlib, python-tempita, python-yaml."
- Wait for the GUI to be ready/started.
- Deploy this bundle: http://pastebin.ubuntu.com/6411548/
- Check everything is ok, xy annotations work (
   the services are vertically aligned), wordpress has
   customized constraints, mysql customized options and
   two units.
- No try to deploy the same bundle again, you
   will see a "services already there" kind of error.

https://code.launchpad.net/~frankban/charms/precise/juju-gui/install-yaml/+merge/195113

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/26190043/

Affected files (+30, -25 lines):
   A [revision details]
   M hooks/backend.py
   M hooks/install
   M hooks/utils.py
   M revision
   M server-requirements.pip
   M tests/test_backends.py

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision:
<email address hidden>
+New revision:
<email address hidden>

Index: revision
=== modified file 'revision'
--- revision 2013-11-13 16:36:02 +0000
+++ revision 2013-11-13 18:01:03 +0000
@@ -1,1 +1,1 @@
-97
+98

Index: server-requirements.pip
=== modified file 'server-requirements.pip'
--- server-requirements.pip 2013-11-13 17:45:50 +0000
+++ server-requirements.pip 2013-11-13 18:01:03 +0000
@@ -18,6 +18,8 @@

  # Note: the order of the following dependencies is important! The last ones
  # depends on the previous.
+# The python-bzrlib and python-yaml dependencies are installed using apt in
+# the charm hooks.

  futures==2.1.4
  tornado==3.1.1

Index: hooks/backend.py
=== modified file 'hooks/backend.py'
--- hooks/backend.py 2013-10-08 07:46:11 +0000
+++ hooks/backend.py 2013-11-13 18:01:03 +0000
@@ -106,8 +106,7 @@

  class GoMixin(object):
      """Manage the real Go juju-core backend."""
-
- debs = ('python-yaml',)
+ pass

  class GuiMixin(object):

Index: hooks/install
=== modified file 'hooks/install'
--- hooks/install 2013-10-07 14:45:00 +0000
+++ hooks/install 2013-11-13 18:01:03 +0000
@@ -20,10 +20,6 @@
  import errno
  import os

-from charmhelpers import (
- get_config,
- log,
-)
  from shelltoolbox import (
      apt_get_install,
      run,
@@ -32,1...

[lp:~frankban/charms/precise/juju-gui/install-yaml]

129. By Francesco Banconi on 2013-11-13

Fix typo.

[Francesco Banconi (frankban)]

Please take a look.

https://codereview.appspot.com/26190043/

[Gary Poster (gary)]

LGTM and QAOK. Thank you!

https://codereview.appspot.com/26190043/

[Francesco Banconi (frankban)]

*** Submitted:

Fix base Python dependencies.

The install hook imported charmhelpers
before installing python-yaml, and
charmhelpers imports from yaml.

This very old bug has been hidden by the
fact that python-yaml is automatically
installed in ec2/lxc instances, but showed
itself when using the manual provider.

Copy/pasting and integrating the QA from
my last branch, a double check can help
before the release.
QA:
- Bootstrap a Juju environment with --debug.
- Deploy and expose the GUI (make deploy).
- Check juju debug-log (or
   less ~/.juju/local/log/unit-juju-gui-0.log if you
   used lxc): you should see the following message
   at the beginning of the install hook log:
   "Installing base Python dependencies: python-apt,
   python-launchpadlib, python-tempita, python-yaml."
- Wait for the GUI to be ready/started.
- Deploy this bundle: http://pastebin.ubuntu.com/6411548/
- Check everything is ok, xy annotations work (
   the services are vertically aligned), wordpress has
   customized constraints, mysql customized options and
   two units.
- No try to deploy the same bundle again, you
   will see a "services already there" kind of error.

R=gary.poster
CC=
https://codereview.appspot.com/26190043

https://codereview.appspot.com/26190043/

=== modified file 'hooks/backend.py'
--- hooks/backend.py	2013-10-08 07:46:11 +0000
+++ hooks/backend.py	2013-11-13 18:37:19 +0000
@@ -106,8 +106,7 @@
 
 class GoMixin(object):
     """Manage the real Go juju-core backend."""
-
-    debs = ('python-yaml',)
+    pass
 
 
 class GuiMixin(object):

=== modified file 'hooks/install'
--- hooks/install	2013-10-07 14:45:00 +0000
+++ hooks/install	2013-11-13 18:37:19 +0000
@@ -20,10 +20,6 @@
 import errno
 import os
 
-from charmhelpers import (
-    get_config,
-    log,
-)
 from shelltoolbox import (
     apt_get_install,
     run,
@@ -32,13 +28,21 @@
 
 # Python dependencies must be installed here so that the charm can import and
 # use required libraries.
-PYTHON_DEPENDENCIES = ('python-apt', 'python-launchpadlib', 'python-tempita')
+PYTHON_DEPENDENCIES = (
+    'python-apt', 'python-launchpadlib', 'python-tempita', 'python-yaml')
 
-log('Installing base Python dependnecies: {}.'.format(
+run('juju-log', '--', 'Installing base Python dependencies: {}.'.format(
     ', '.join(PYTHON_DEPENDENCIES)))
 apt_get_install(*PYTHON_DEPENDENCIES)
 
 
+# The charmhelpers module depends on python-yaml and must be imported only
+# after the installation of the Python dependencies above.
+from charmhelpers import (
+    get_config,
+    log,
+)
+
 from utils import (
     config_json,
     log_hook,

=== modified file 'hooks/utils.py'
--- hooks/utils.py	2013-11-12 12:23:25 +0000
+++ hooks/utils.py	2013-11-13 18:37:19 +0000
@@ -74,11 +74,20 @@
 import urlparse
 
 import apt
+from launchpadlib.launchpad import Launchpad
 import tempita
+import yaml
 
-from launchpadlib.launchpad import Launchpad
+from charmhelpers import (
+    get_config,
+    log,
+    RESTART,
+    service_control,
+    START,
+    STOP,
+    unit_get,
+)
 from shelltoolbox import (
-    Serializer,
     apt_get_install,
     command,
     environ,
@@ -86,17 +95,9 @@
     run,
     script_name,
     search_file,
+    Serializer,
     su,
 )
-from charmhelpers import (
-    RESTART,
-    START,
-    STOP,
-    get_config,
-    log,
-    service_control,
-    unit_get,
-)
 
 
 AGENT = 'juju-api-agent'
@@ -173,7 +174,6 @@
     # The JUJU_API_ADDRESSES environment variable is not included in the hooks
     # context in older releases of juju-core.  Retrieve it from the machiner
     # agent file instead.
-    import yaml  # python-yaml is only installed if juju-core is used.
     if unit_dir is None:
         base_dir = os.path.join(CURRENT_DIR, '..', '..')
     else:

=== modified file 'revision'
--- revision	2013-11-13 16:36:02 +0000
+++ revision	2013-11-13 18:37:19 +0000
@@ -1,1 +1,1 @@
-97
+98

=== modified file 'server-requirements.pip'
--- server-requirements.pip	2013-11-13 17:45:50 +0000
+++ server-requirements.pip	2013-11-13 18:37:19 +0000
@@ -18,6 +18,8 @@
 
 # Note: the order of the following dependencies is important! The last ones
 # depends on the previous.
+# The python-bzrlib and python-yaml dependencies are installed using apt in
+# the charm hooks.
 
 futures==2.1.4
 tornado==3.1.1

=== modified file 'tests/test_backends.py'
--- tests/test_backends.py	2013-10-07 14:17:11 +0000
+++ tests/test_backends.py	2013-11-13 18:37:19 +0000
@@ -33,12 +33,10 @@
 
 
 EXPECTED_PYTHON_LEGACY_DEBS = ('apache2', 'curl', 'haproxy', 'openssl')
-EXPECTED_GO_LEGACY_DEBS = (
-    'apache2', 'curl', 'haproxy', 'openssl', 'python-yaml')
+EXPECTED_GO_LEGACY_DEBS = ('apache2', 'curl', 'haproxy', 'openssl')
 EXPECTED_PYTHON_BUILTIN_DEBS = (
     'curl', 'openssl', 'python-bzrlib', 'python-pip')
-EXPECTED_GO_BUILTIN_DEBS = (
-    'curl', 'openssl', 'python-bzrlib', 'python-pip', 'python-yaml')
+EXPECTED_GO_BUILTIN_DEBS = ('curl', 'openssl', 'python-bzrlib', 'python-pip')
 
 simulate_pyjuju = mock.patch('utils.legacy_juju', mock.Mock(return_value=True))
 simulate_juju_core = mock.patch(