Merge lp:~rharding/charms/precise/juju-gui/fix-log-test into lp:~juju-gui/charms/precise/juju-gui/trunk

[Richard Harding]

Remove existing Log handers when setupLogging.

- If there are existing handlers on the root logger then the
logging.basicConfig is a noop. Changes we expect to see will not occur.

[Richard Harding (rharding)]

Reviewers: mp+201453_code.launchpad.net,

Message:
Please take a look.

Description:
Remove existing Log handers when setupLogging.

- If there are existing handlers on the root logger then the
logging.basicConfig is a noop. Changes we expect to see will not occur.

https://code.launchpad.net/~rharding/charms/precise/juju-gui/fix-log-test/+merge/201453

(do not edit description out of merge proposal)

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

Affected files (+11, -0 lines):
   A [revision details]
   M hooks/utils.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: hooks/utils.py
=== modified file 'hooks/utils.py'
--- hooks/utils.py 2013-11-15 14:56:04 +0000
+++ hooks/utils.py 2014-01-13 16:39:51 +0000
@@ -319,6 +319,15 @@
      global results_log
      if results_log is not None:
          return
+
+ # Make sure that the root logger isn't configured already. If it does,
+ # this basicConfig will be a noop and not setup the expected file
handler
+ # on the logger.
+ root_logger = logging.getLogger()
+ if root_logger.handlers:
+ for handler in root_logger.handlers:
+ root_logger.removeHandler(handler)
+
      config = get_config()
      logging.basicConfig(
          filename=config['command-log-file'],

[Francesco Banconi (frankban)]

LGTM with the suggested changes (especially the second one).
Thank you!

https://codereview.appspot.com/51280047/diff/1/hooks/utils.py
File hooks/utils.py (right):

https://codereview.appspot.com/51280047/diff/1/hooks/utils.py#newcode327
hooks/utils.py:327: if root_logger.handlers:
This check can be omitted.

https://codereview.appspot.com/51280047/diff/1/hooks/utils.py#newcode328
hooks/utils.py:328: for handler in root_logger.handlers:
I'd suggest to use:
for handler in root_logger.handlers[:]:
so that we avoid modifying in place the list we are looping over.

https://codereview.appspot.com/51280047/

=== modified file 'hooks/utils.py'
--- hooks/utils.py	2014-01-09 21:48:27 +0000
+++ hooks/utils.py	2014-01-13 16:42:12 +0000
@@ -319,6 +319,15 @@
     global results_log
     if results_log is not None:
         return
+
+    # Make sure that the root logger isn't configured already. If it does,
+    # this basicConfig will be a noop and not setup the expected file handler
+    # on the logger.
+    root_logger = logging.getLogger()
+    if root_logger.handlers:
+        for handler in root_logger.handlers:
+            root_logger.removeHandler(handler)
+
     config = get_config()
     logging.basicConfig(
         filename=config['command-log-file'],