yellow team mailing list archive

[Brad Crittenden <bac@xxxxxxxxxxxxx>]

Brad Crittenden has proposed merging lp:~bac/charms/precise/juju-gui/1086790 into lp:~juju-gui/charms/precise/juju-gui/trunk.

Requested reviews:
  Juju GUI Hackers (juju-gui)
Related bugs:
  Bug #1086790 in juju-gui: "charm does not include command stdout or stderr"
  https://bugs.launchpad.net/juju-gui/+bug/1086790

For more details, see:
https://code.launchpad.net/~bac/charms/precise/juju-gui/1086790/+merge/138823

Add command-level logging in the hooks.  Also for the run('make') command, stdout and stderr are redirected to a separate file so that progress can be monitored as it occurs.  None of the other commands are long running to require that level of detail.

Sorry for not using Rietveld but I could not get lbox to work with this target.
-- 
https://code.launchpad.net/~bac/charms/precise/juju-gui/1086790/+merge/138823
Your team Juju GUI Hackers is requested to review the proposed merge of lp:~bac/charms/precise/juju-gui/1086790 into lp:~juju-gui/charms/precise/juju-gui/trunk.

=== modified file 'config.yaml'
--- config.yaml	2012-12-03 10:02:45 +0000
+++ config.yaml	2012-12-07 19:37:21 +0000
@@ -29,6 +29,13 @@
     default: sample
   juju-gui-console-enabled:
     description: |
-      Whether or not the browser's console should be enabled.
+      Whether or not the console should be enabled for the browser.
     type: boolean
     default: false
+  command-log-file:
+    description: |
+      The log file where stdout and stderr should be sent for all
+      commands that are run by charm hooks.
+    type: string
+    default: /var/log/juju/juju-gui.log
+

=== modified file 'hooks/install'
--- hooks/install	2012-11-30 17:00:51 +0000
+++ hooks/install	2012-12-07 19:37:21 +0000
@@ -1,7 +1,12 @@
 #!/usr/bin/env python2
-
-from subprocess import check_call
-
+#-*- python -*-
+
+import os.path
+from subprocess import (
+    CalledProcessError,
+    check_call,
+    )
+import tempfile
 
 # python-shelltoolbox is installed as a dependency of python-charmhelpers.
 check_call(['apt-get', 'install', '-y', 'python-charmhelpers'])
@@ -23,34 +28,45 @@
     run,
 )
 
+from utils import cmd_log
+
 
 def fetch(juju_gui_branch, juju_api_branch):
     """Install required dependencies and retrieve Juju/Juju GUI branches."""
     log('Installing dependencies.')
-    install_extra_repositories('ppa:chris-lea/node.js')
-    apt_get_install('bzr', 'imagemagick', 'make', 'nodejs', 'npm', 'zookeeper')
+    cmd_log(install_extra_repositories('ppa:chris-lea/node.js'))
+    cmd_log(
+        apt_get_install('bzr', 'imagemagick', 'make',
+                        'nodejs', 'npm', 'zookeeper'))
     log('Retrieving source checkouts.')
     bzr_checkout = command('bzr', 'co', '--lightweight')
-    bzr_checkout(juju_gui_branch, 'juju-gui')
-    bzr_checkout(juju_api_branch, 'juju')
-
-
-def build():
+    cmd_log(bzr_checkout(juju_gui_branch, 'juju-gui'))
+    cmd_log(bzr_checkout(juju_api_branch, 'juju'))
+
+
+def build(logpath):
     """Set up Juju GUI."""
     log('Building Juju GUI.')
     with cd('juju-gui'):
-        run('make')
+        logdir = os.path.dirname(logpath)
+        fd, name = tempfile.mkstemp(prefix='make-', dir=logdir)
+        log('Output from "make" sent to', name)
+        run('make', stdout=fd, stderr=fd)
 
 
 def main():
     config = get_config()
     fetch(config['juju-gui-branch'], config['juju-api-branch'])
-    build()
+    build(config['command-log-file'])
 
 
 if __name__ == '__main__':
     log_entry()
     try:
         main()
+    except CalledProcessError as e:
+        log('Exception caught:')
+        log(e.output)
+        raise
     finally:
         log_exit()

=== modified file 'hooks/start'
--- hooks/start	2012-12-03 10:02:45 +0000
+++ hooks/start	2012-12-07 19:37:21 +0000
@@ -1,4 +1,5 @@
 #!/usr/bin/env python2
+#-*- python -*-
 
 import json
 import os

=== modified file 'hooks/stop'
--- hooks/stop	2012-12-03 10:02:45 +0000
+++ hooks/stop	2012-12-07 19:37:21 +0000
@@ -1,7 +1,8 @@
 #!/usr/bin/env python2
+#-*- python -*-
 
 from charmhelpers import (
-    get_config,
+	get_config,
     log,
     log_entry,
     log_exit,

=== modified file 'hooks/utils.py'
--- hooks/utils.py	2012-11-29 14:49:41 +0000
+++ hooks/utils.py	2012-12-07 19:37:21 +0000
@@ -1,8 +1,13 @@
 """Juju GUI charm utilities."""
 
 import os
-
-from shelltoolbox import search_file
+import logging
+
+from shelltoolbox import (
+    search_file,
+    )
+
+from charmhelpers import get_config
 
 
 def get_zookeeper_address(agent_file_path):
@@ -30,3 +35,26 @@
     contents = open(template_path).read()
     with open(destination, 'w') as stream:
         stream.write(contents % context)
+
+
+results_log = None
+
+def _setupLogging():
+    global results_log
+    if results_log is not None:
+        return
+    config = get_config()
+    logging.basicConfig(
+        filename=config['command-log-file'],
+        level=logging.INFO,
+        format="%(asctime)s: %(name)s@%(levelname)s %(message)s")
+    results_log = logging.getLogger('juju-gui')
+
+
+def cmd_log(results):
+    global results_log
+    if not results:
+        return
+    if results_log is None:
+        _setupLogging()
+    results_log.info('\n' + results)

=== modified file 'revision'
--- revision	2012-11-30 17:36:15 +0000
+++ revision	2012-12-07 19:37:21 +0000
@@ -1,1 +1,1 @@
-12
+13

=== modified file 'tests/deploy.test'
--- tests/deploy.test	2012-11-30 17:00:51 +0000
+++ tests/deploy.test	2012-12-07 19:37:21 +0000
@@ -1,4 +1,5 @@
 #!/usr/bin/env python2
+#-*- python -*-
 
 import time
 import unittest

=== modified file 'tests/test_utils.py'
--- tests/test_utils.py	2012-11-29 14:49:41 +0000
+++ tests/test_utils.py	2012-12-07 19:37:21 +0000
@@ -3,8 +3,11 @@
 import os
 import tempfile
 import unittest
+import charmhelpers
+from simplejson import dumps
 
 from utils import (
+    cmd_log,
     get_zookeeper_address,
     render_to_file,
 )
@@ -44,6 +47,25 @@
         expected = self.template_contents % context
         self.assertEqual(expected, self.destination_file.read())
 
+class CmdLogTest(unittest.TestCase):
+    def setUp(self):
+        # Patch the charmhelpers 'command', which powers get_config.  The
+        # result of this is the mock_config dictionary will be returned.
+        # The monkey patch is undone at the end of the test.
+        self.command = charmhelpers.command
+        fd, self.log_file_name = tempfile.mkstemp()
+        os.close(fd)
+        mock_config = {'command-log-file': self.log_file_name}
+        charmhelpers.command = lambda *args: lambda: dumps(mock_config)
+
+    def tearDown(self):
+        charmhelpers.command = self.command
+
+    def test_contents_logged(self):
+        cmd_log('foo')
+        line = open(self.log_file_name, 'r').read()
+        self.assertTrue(line.endswith(': juju-gui@INFO \nfoo\n'))
+
 
 if __name__ == '__main__':
     unittest.main(verbosity=2)

=== modified file 'tests/unit.test'
--- tests/unit.test	2012-11-21 14:51:05 +0000
+++ tests/unit.test	2012-12-07 19:37:21 +0000
@@ -1,4 +1,5 @@
 #!/usr/bin/env python2
+#-*- python -*-
 
 import os
 import sys