Merge lp:~bigkevmcd/landscape-client/517454-incorrect-log-file-location into lp:~landscape/landscape-client/trunk

[Kevin McDermott]

[Thomas Herve (therve)]

[1] Pep8:
landscape/sysinfo/tests/test_deployment.py:20:5: E301 expected 1 blank line, found 0
landscape/sysinfo/tests/test_deployment.py:22:9: E301 expected 1 blank line, found 0
landscape/sysinfo/tests/test_deployment.py:63:5: E301 expected 1 blank line, found 0
landscape/sysinfo/tests/test_deployment.py:67:5: E301 expected 1 blank line, found 0
landscape/sysinfo/tests/test_deployment.py:69:5: E301 expected 1 blank line, found 0
landscape/sysinfo/tests/test_deployment.py:71:5: E301 expected 1 blank line, found 0
landscape/sysinfo/tests/test_deployment.py:73:5: E301 expected 1 blank line, found 0
landscape/sysinfo/tests/test_deployment.py:144:9: E301 expected 1 blank line, found 0
landscape/sysinfo/tests/test_deployment.py:261:1: W291 trailing whitespace
landscape/sysinfo/tests/test_deployment.py:263:5: E301 expected 1 blank line, found 2
landscape/sysinfo/sysinfo.py:18:1: W291 trailing whitespace
landscape/sysinfo/sysinfo.py:92:1: W291 trailing whitespace
landscape/sysinfo/sysinfo.py:236:22: E111 indentation is not a multiple of four
landscape/sysinfo/tests/test_sysinfo.py:80:9: E301 expected 1 blank line, found 0
landscape/sysinfo/tests/test_sysinfo.py:81:13: E301 expected 1 blank line, found 0
landscape/sysinfo/tests/test_sysinfo.py:83:13: E301 expected 1 blank line, found 0
landscape/sysinfo/tests/test_sysinfo.py:85:13: E301 expected 1 blank line, found 0
landscape/sysinfo/tests/test_sysinfo.py:120:9: E301 expected 1 blank line, found 0
landscape/sysinfo/tests/test_sysinfo.py:121:13: E301 expected 1 blank line, found 0
landscape/sysinfo/tests/test_sysinfo.py:123:13: E301 expected 1 blank line, found 0
landscape/sysinfo/tests/test_sysinfo.py:126:9: E301 expected 1 blank line, found 0
landscape/sysinfo/tests/test_sysinfo.py:127:13: E301 expected 1 blank line, found 0
landscape/sysinfo/tests/test_sysinfo.py:129:13: E301 expected 1 blank line, found 0
landscape/sysinfo/tests/test_sysinfo.py:151:9: E301 expected 1 blank line, found 0
landscape/sysinfo/tests/test_sysinfo.py:152:13: E301 expected 1 blank line, found 0
landscape/sysinfo/tests/test_sysinfo.py:154:13: E301 expected 1 blank line, found 0
landscape/sysinfo/tests/test_sysinfo.py:170:9: E301 expected 1 blank line, found 0
landscape/sysinfo/tests/test_sysinfo.py:171:13: E301 expected 1 blank line, found 0
landscape/sysinfo/tests/test_sysinfo.py:173:13: E301 expected 1 blank line, found 0
landscape/sysinfo/tests/test_sysinfo.py:177:13: E301 expected 1 blank line, found 0
landscape/sysinfo/tests/test_sysinfo.py:179:13: E301 expected 1 blank line, found 0
landscape/sysinfo/tests/test_sysinfo.py:199:9: E301 expected 1 blank line, found 0
landscape/sysinfo/tests/test_sysinfo.py:200:13: E301 expected 1 blank line, found 0
landscape/sysinfo/tests/test_sysinfo.py:202:13: E301 expected 1 blank line, found 0
landscape/sysinfo/tests/test_sysinfo.py:214:1: E302 expected 2 blank lines, found 1
landscape/sysinfo/tests/test_sysinfo.py:300:31: W291 trailing whitespace
landscape/sysinfo/tests/test_sysinfo.py:329:37: E202 whitespace before ')'
landscape/sysinfo/tests/test_sysinfo.py:338:31: W291 trailing whitespace
landscape/sysinfo/tests/test_sysinfo.py:341:37: E202 whitespace before ')'
landscape/sysi...

review: Approve

[Free Ekanayaka (free.ekanayaka)]

Nice fix and tests! +1

[1]

+ def test_exception_running_as_privileged_user(self):

This test is missing a docstring.

review: Approve

=== modified file 'landscape/sysinfo/deployment.py'
--- landscape/sysinfo/deployment.py	2008-09-18 18:42:22 +0000
+++ landscape/sysinfo/deployment.py	2010-02-05 15:57:13 +0000
@@ -60,14 +60,21 @@
                            % (plugin_name.lower(), plugin_name))()
                 for plugin_name in plugins]
 
-
-def setup_logging(landscape_dir=None):
+def get_landscape_log_directory(landscape_dir=None):
+    """
+    Work out the correct path to store logs in depending on the effective
+    user id of the current process.
+    """
     if landscape_dir is None:
         if os.getuid() == 0:
             landscape_dir = "/var/log/landscape"
         else:
             landscape_dir = os.path.expanduser("~/.landscape")
-
+    return landscape_dir
+
+
+def setup_logging(landscape_dir=None):
+    landscape_dir = get_landscape_log_directory(landscape_dir)
     logger = getLogger("landscape-sysinfo")
     logger.propagate = False
     if not os.path.isdir(landscape_dir):

=== modified file 'landscape/sysinfo/sysinfo.py'
--- landscape/sysinfo/sysinfo.py	2008-09-03 16:59:55 +0000
+++ landscape/sysinfo/sysinfo.py	2010-02-05 15:57:13 +0000
@@ -1,6 +1,7 @@
 import textwrap
 from logging import getLogger
 import math
+import os
 
 from twisted.python.failure import Failure
 
@@ -110,10 +111,12 @@
         log_failure(failure, message, logger=logger)
 
     def _report_error_note(self, result):
+        from landscape.sysinfo.deployment import get_landscape_log_directory
+        path = os.path.join(get_landscape_log_directory(), "sysinfo.log")
         if self._plugin_error:
             self.add_note(
                 "There were exceptions while processing one or more plugins. "
-                "See ~/.landscape/sysinfo.log for more information.")
+                "See %s for more information." % path)
         return result
 
 

=== modified file 'landscape/sysinfo/tests/test_deployment.py'
--- landscape/sysinfo/tests/test_deployment.py	2009-08-07 22:36:33 +0000
+++ landscape/sysinfo/tests/test_deployment.py	2010-02-05 15:57:13 +0000
@@ -6,7 +6,8 @@
 from twisted.internet.defer import Deferred
 
 from landscape.sysinfo.deployment import (
-    SysInfoConfiguration, ALL_PLUGINS, run, setup_logging)
+    SysInfoConfiguration, ALL_PLUGINS, run, setup_logging,
+    get_landscape_log_directory)
 from landscape.sysinfo.testplugin import TestPlugin
 from landscape.sysinfo.sysinfo import SysInfoPluginRegistry
 from landscape.sysinfo.load import Load
@@ -192,6 +193,25 @@
         self.assertEquals(reactor.scheduled_calls, [(0, reactor.stop, (), {})])
         return self.assertFailure(d, ZeroDivisionError)
 
+    def test_get_landscape_log_directory_unprivileged(self):
+        """
+        If landscape-sysinfo is running as a non-privileged user the
+        log directory is stored in their home directory.
+        """
+        self.assertEquals(get_landscape_log_directory(),
+                          os.path.expanduser("~/.landscape"))
+
+    def test_get_landscape_log_directory_privileged(self):
+        """
+        If landscape-sysinfo is running as a privileged user, then the logs
+        should be stored in the system-wide log directory.
+        """
+        uid_mock = self.mocker.replace("os.getuid")
+        uid_mock()
+        self.mocker.result(0)
+        self.mocker.replay()
+        self.assertEquals(get_landscape_log_directory(), "/var/log/landscape")
+
     def test_wb_logging_setup(self):
         """
         setup_logging sets up a "landscape-sysinfo" logger which rotates every

=== modified file 'landscape/sysinfo/tests/test_sysinfo.py'
--- landscape/sysinfo/tests/test_sysinfo.py	2008-09-03 16:59:55 +0000
+++ landscape/sysinfo/tests/test_sysinfo.py	2010-02-05 15:57:13 +0000
@@ -1,5 +1,6 @@
 from cStringIO import StringIO
 from logging import getLogger, StreamHandler
+import os
 
 from twisted.internet.defer import Deferred, succeed, fail
 
@@ -107,7 +108,7 @@
 
     plugin_exception_message = (
         "There were exceptions while processing one or more plugins. "
-        "See ~/.landscape/sysinfo.log for more information.")
+        "See %s/sysinfo.log for more information.")
 
     def test_plugins_run_after_synchronous_error(self):
         """
@@ -139,9 +140,11 @@
         self.assertIn(message, log)
         self.assertIn("1/0", log)
         self.assertIn("ZeroDivisionError", log)
+
+        path = os.path.expanduser("~/.landscape")
         self.assertEquals(
             self.sysinfo.get_notes(),
-            [self.plugin_exception_message])
+            [self.plugin_exception_message % path])
 
     def test_asynchronous_errors_logged(self):
         self.log_helper.ignore_errors(ZeroDivisionError)
@@ -157,9 +160,10 @@
         message = "BadPlugin plugin raised an exception."
         self.assertIn(message, log)
         self.assertIn("ZeroDivisionError: yay", log)
+        path = os.path.expanduser("~/.landscape")
         self.assertEquals(
             self.sysinfo.get_notes(),
-            [self.plugin_exception_message])
+            [self.plugin_exception_message % path])
 
     def test_multiple_exceptions_get_one_note(self):
         self.log_helper.ignore_errors(ZeroDivisionError)
@@ -181,11 +185,31 @@
         self.sysinfo.add(plugin2)
         self.sysinfo.run()
 
-        self.assertEquals(
-            self.sysinfo.get_notes(),
-            [self.plugin_exception_message])
-        
-
+        path = os.path.expanduser("~/.landscape")
+        self.assertEquals(
+            self.sysinfo.get_notes(),
+            [self.plugin_exception_message % path])
+
+    def test_exception_running_as_privileged_user(self):
+        uid_mock = self.mocker.replace("os.getuid")
+        uid_mock()
+        self.mocker.result(0)
+        self.mocker.replay()
+        self.log_helper.ignore_errors(ZeroDivisionError)
+        class AsyncBadPlugin(object):
+            def register(self, registry):
+                pass
+            def run(self):
+                return fail(ZeroDivisionError("Hi"))
+
+        plugin = AsyncBadPlugin()
+        self.sysinfo.add(plugin)
+        self.sysinfo.run()
+
+        path = "/var/log/landscape"
+        self.assertEquals(
+            self.sysinfo.get_notes(),
+            [self.plugin_exception_message % path])
 
 class FormatTest(LandscapeTest):
 
@@ -345,4 +369,4 @@
             """\
 Z=> I do believe that a very long note, such as
     one that is longer than about 50 characters,
-    should wrap at the specified width.""")
\ No newline at end of file
+    should wrap at the specified width.""")