From bc15eb4e6b051f557567aca767a1160939c0f6d7 Mon Sep 17 00:00:00 2001 From: Matthieu Coudron Date: Wed, 17 Dec 2014 23:39:10 +0100 Subject: [PATCH 1/5] i3pystatus creates a logger that can be imported via import i3pystatus.logger . Thus when modules enable the 'enable_log' setting, it should log i3pystatus errors to a file called '.i3pystatus-'. This commit only solves the case when email_client was called in the email module and would output things into stderr/stdout. --- i3pystatus/__init__.py | 12 ++++++++++++ i3pystatus/core/command.py | 37 +++++++++++++++++++++++++++++++++++++ i3pystatus/mail/__init__.py | 4 +++- 3 files changed, 52 insertions(+), 1 deletion(-) create mode 100644 i3pystatus/core/command.py diff --git a/i3pystatus/__init__.py b/i3pystatus/__init__.py index d18ca03..df270a2 100644 --- a/i3pystatus/__init__.py +++ b/i3pystatus/__init__.py @@ -5,6 +5,17 @@ from i3pystatus.core.modules import Module, IntervalModule from i3pystatus.core.settings import SettingsBase from i3pystatus.core.util import formatp +import logging +import os + +h=logging.FileHandler(".i3pystatus-" + str(os.getpid())); + +logger = logging.getLogger("i3pystatus") +logger.addHandler(h) +logger.setLevel(logging.DEBUG) +#~/.i3pystatus-- +logger.error("Start !") + __path__ = extend_path(__path__, __name__) __all__ = [ @@ -12,6 +23,7 @@ __all__ = [ "Module", "IntervalModule", "SettingsBase", "formatp", + "logger", ] diff --git a/i3pystatus/core/command.py b/i3pystatus/core/command.py new file mode 100644 index 0000000..a1eb41a --- /dev/null +++ b/i3pystatus/core/command.py @@ -0,0 +1,37 @@ +from subprocess import check_output, CalledProcessError +import subprocess +from i3pystatus import logger +# class Command(Object): + +# ,*args +def run_through_shell(command,enable_log): + """ + Retrieves output of shell command + Returns tuple boolean (success)/ string (error msg, output) + """ + result=False + try: + #stderr=subprocess.STDOUT + #stderr=subprocess.PIPE + # with subprocess.Popen() as proc: + # logger.error(proc.stderr.read()) + proc = subprocess.Popen(command,stderr=subprocess.PIPE,stdout=subprocess.PIPE) + + out, stderr = proc.communicate() + if stderr and enable_log: + logger.error(stderr) + # out = check_output(command, stderr=subprocess.STDOUT, shell=True) + result=True + # color = self.color + except CalledProcessError as e: + out = e.output + # color = self.error_color + + out = out.decode("UTF-8").replace("\n", " ") + try: + if out[-1] == " ": + out = out[:-1] + except: + out = "" + + return result, out diff --git a/i3pystatus/mail/__init__.py b/i3pystatus/mail/__init__.py index abf0ba1..cc9658b 100644 --- a/i3pystatus/mail/__init__.py +++ b/i3pystatus/mail/__init__.py @@ -2,6 +2,7 @@ import subprocess from i3pystatus import SettingsBase, IntervalModule from i3pystatus.core.util import internet, require +from i3pystatus.core.command import run_through_shell class Backend(SettingsBase): @@ -69,7 +70,8 @@ class Mail(IntervalModule): def on_leftclick(self): if self.email_client: - subprocess.Popen(self.email_client.split()) + run_through_shell(self.email_client,self.enable_log) + def on_rightclick(self): self.run() From 69c1cd6460b9c7e6a37a01e086d26b49ecb1798c Mon Sep 17 00:00:00 2001 From: Matthieu Coudron Date: Wed, 17 Dec 2014 23:51:10 +0100 Subject: [PATCH 2/5] Various pep8 and comments removal + converted shell module to use run_through_shell command --- i3pystatus/__init__.py | 5 ++--- i3pystatus/core/command.py | 25 ++++++++++--------------- i3pystatus/mail/__init__.py | 6 +----- i3pystatus/shell.py | 19 +++---------------- 4 files changed, 16 insertions(+), 39 deletions(-) diff --git a/i3pystatus/__init__.py b/i3pystatus/__init__.py index df270a2..8845385 100644 --- a/i3pystatus/__init__.py +++ b/i3pystatus/__init__.py @@ -8,13 +8,12 @@ from i3pystatus.core.util import formatp import logging import os -h=logging.FileHandler(".i3pystatus-" + str(os.getpid())); +h = logging.FileHandler(".i3pystatus-" + str(os.getpid()), delay=True) logger = logging.getLogger("i3pystatus") logger.addHandler(h) logger.setLevel(logging.DEBUG) -#~/.i3pystatus-- -logger.error("Start !") + __path__ = extend_path(__path__, __name__) diff --git a/i3pystatus/core/command.py b/i3pystatus/core/command.py index a1eb41a..e0e8e47 100644 --- a/i3pystatus/core/command.py +++ b/i3pystatus/core/command.py @@ -1,28 +1,23 @@ -from subprocess import check_output, CalledProcessError +from subprocess import CalledProcessError import subprocess from i3pystatus import logger -# class Command(Object): -# ,*args -def run_through_shell(command,enable_log): + +def run_through_shell(command, enable_log, enable_shell=False): """ Retrieves output of shell command Returns tuple boolean (success)/ string (error msg, output) """ - result=False + result = False try: - #stderr=subprocess.STDOUT - #stderr=subprocess.PIPE - # with subprocess.Popen() as proc: - # logger.error(proc.stderr.read()) - proc = subprocess.Popen(command,stderr=subprocess.PIPE,stdout=subprocess.PIPE) - + proc = subprocess.Popen(command, stderr=subprocess.PIPE, stdout=subprocess.PIPE, shell=enable_shell) + out, stderr = proc.communicate() if stderr and enable_log: logger.error(stderr) - # out = check_output(command, stderr=subprocess.STDOUT, shell=True) - result=True - # color = self.color + + result = True + except CalledProcessError as e: out = e.output # color = self.error_color @@ -34,4 +29,4 @@ def run_through_shell(command,enable_log): except: out = "" - return result, out + return out, result diff --git a/i3pystatus/mail/__init__.py b/i3pystatus/mail/__init__.py index cc9658b..afe6c25 100644 --- a/i3pystatus/mail/__init__.py +++ b/i3pystatus/mail/__init__.py @@ -1,7 +1,4 @@ -import subprocess - from i3pystatus import SettingsBase, IntervalModule -from i3pystatus.core.util import internet, require from i3pystatus.core.command import run_through_shell @@ -70,8 +67,7 @@ class Mail(IntervalModule): def on_leftclick(self): if self.email_client: - run_through_shell(self.email_client,self.enable_log) - + run_through_shell(self.email_client, self.enable_log) def on_rightclick(self): self.run() diff --git a/i3pystatus/shell.py b/i3pystatus/shell.py index 571521f..12de74b 100644 --- a/i3pystatus/shell.py +++ b/i3pystatus/shell.py @@ -1,5 +1,5 @@ from i3pystatus import IntervalModule -from subprocess import check_output, CalledProcessError +from i3pystatus.core.command import run_through_shell class Shell(IntervalModule): @@ -19,21 +19,8 @@ class Shell(IntervalModule): required = ("command",) def run(self): - try: - out = check_output(self.command, shell=True) - color = self.color - except CalledProcessError as e: - out = e.output - color = self.error_color - - out = out.decode("UTF-8").replace("\n", " ") - try: - if out[-1] == " ": - out = out[:-1] - except: - out = "" - + out, success = run_through_shell(self.command, self.enable_log, enable_shell=True) self.output = { "full_text": out, - "color": color + "color": self.color if success else self.error_color } From 49a0f01c76f1d2028f8a9f1aa9b2538288c8d82d Mon Sep 17 00:00:00 2001 From: Matthieu Coudron Date: Thu, 18 Dec 2014 10:46:53 +0100 Subject: [PATCH 3/5] Added missing r to IntevalSettings in test_core_modules --- tests/test_core_modules.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_core_modules.py b/tests/test_core_modules.py index 84b40cb..362dab3 100644 --- a/tests/test_core_modules.py +++ b/tests/test_core_modules.py @@ -20,9 +20,9 @@ class IntervalModuleMetaTest(unittest.TestCase): (('option', 'desc'), 'interval')) def test_settings_with_interval(self): - class SettingsInteval(IntervalModule): + class SettingsInterval(IntervalModule): settings = ('option', 'interval') - self.assertEqual(SettingsInteval.settings, ('option', 'interval')) + self.assertEqual(SettingsInterval.settings, ('option', 'interval')) def test_settings_with_interval_desc(self): class SetttingsIntervalDesc(IntervalModule): From e9df3a82de5251bbbd8ee31e907f602a4aee1797 Mon Sep 17 00:00:00 2001 From: Matthieu Coudron Date: Thu, 18 Dec 2014 18:34:57 +0100 Subject: [PATCH 4/5] Make sure that we use python3 pep8 --- ci-build.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ci-build.sh b/ci-build.sh index db59a73..61195aa 100755 --- a/ci-build.sh +++ b/ci-build.sh @@ -2,13 +2,13 @@ python3 --version py.test --version -pep8 --version +python3 -mpep8 --version # Target directory for all build files BUILD=${1:-ci-build} mkdir -p $BUILD -pep8 --ignore E501 i3pystatus tests +python3 -mpep8 --ignore E501 i3pystatus tests # Check that the setup.py script works rm -rf ${BUILD}/test-install ${BUILD}/test-install-bin From 9e3f128a15729646dfd7a725f25938ec5d52f5ab Mon Sep 17 00:00:00 2001 From: Matthieu Coudron Date: Thu, 18 Dec 2014 18:42:27 +0100 Subject: [PATCH 5/5] This commit improves the logging system: instead of setting a boolean to enable logging, the user sets a logging level (per module). By default only critical errors are logged (ie nothing for now). Also adds a test for the function run_through_shell --- i3pystatus/__init__.py | 3 +-- i3pystatus/core/command.py | 35 ++++++++++++++++------------------- i3pystatus/core/settings.py | 8 ++++++-- i3pystatus/mail/__init__.py | 4 +++- i3pystatus/shell.py | 18 +++++++++++++++--- tests/test_core_modules.py | 3 ++- tests/test_shell.py | 24 ++++++++++++++++++++++++ 7 files changed, 67 insertions(+), 28 deletions(-) create mode 100644 tests/test_shell.py diff --git a/i3pystatus/__init__.py b/i3pystatus/__init__.py index 8845385..6d578cb 100644 --- a/i3pystatus/__init__.py +++ b/i3pystatus/__init__.py @@ -12,7 +12,7 @@ h = logging.FileHandler(".i3pystatus-" + str(os.getpid()), delay=True) logger = logging.getLogger("i3pystatus") logger.addHandler(h) -logger.setLevel(logging.DEBUG) +logger.setLevel(logging.CRITICAL) __path__ = extend_path(__path__, __name__) @@ -22,7 +22,6 @@ __all__ = [ "Module", "IntervalModule", "SettingsBase", "formatp", - "logger", ] diff --git a/i3pystatus/core/command.py b/i3pystatus/core/command.py index e0e8e47..3a7813b 100644 --- a/i3pystatus/core/command.py +++ b/i3pystatus/core/command.py @@ -1,32 +1,29 @@ -from subprocess import CalledProcessError +# from subprocess import CalledProcessError import subprocess -from i3pystatus import logger -def run_through_shell(command, enable_log, enable_shell=False): +def run_through_shell(command, enable_shell=False): """ - Retrieves output of shell command - Returns tuple boolean (success)/ string (error msg, output) + Retrieves output of command + Returns tuple success (boolean)/ stdout(string) / stderr (string) + + Don't use this function with programs that outputs lots of data since the output is saved + in one variable """ - result = False + returncode = None try: proc = subprocess.Popen(command, stderr=subprocess.PIPE, stdout=subprocess.PIPE, shell=enable_shell) out, stderr = proc.communicate() - if stderr and enable_log: - logger.error(stderr) + out = out.decode("UTF-8") + stderr = stderr.decode("UTF-8") - result = True + returncode = proc.returncode - except CalledProcessError as e: + except OSError as e: + out = e.strerror + stderr = e.strerror + except subprocess.CalledProcessError as e: out = e.output - # color = self.error_color - out = out.decode("UTF-8").replace("\n", " ") - try: - if out[-1] == " ": - out = out[:-1] - except: - out = "" - - return out, result + return returncode, out, stderr diff --git a/i3pystatus/core/settings.py b/i3pystatus/core/settings.py index eae708c..6d67ae6 100644 --- a/i3pystatus/core/settings.py +++ b/i3pystatus/core/settings.py @@ -1,6 +1,7 @@ from i3pystatus.core.util import KeyConstraintDict from i3pystatus.core.exceptions import ConfigKeyError, ConfigMissingError import inspect +import logging class SettingsBase: @@ -18,7 +19,7 @@ class SettingsBase: """ settings = ( - ("enable_log", "Set to true to log error to .i3pystatus- file"), + ("log_level", "Set to true to log error to .i3pystatus- file"), ) """settings should be tuple containing two types of elements: @@ -31,7 +32,8 @@ class SettingsBase: required = tuple() """required can list settings which are required""" - enable_log = False + log_level = logging.NOTSET + logger = None def __init__(self, *args, **kwargs): def get_argument_dict(args, kwargs): @@ -71,6 +73,8 @@ class SettingsBase: self.__name__ = "{}.{}".format( self.__module__, self.__class__.__name__) + self.logger = logging.getLogger(self.__name__) + self.logger.setLevel(self.log_level) self.init() def init(self): diff --git a/i3pystatus/mail/__init__.py b/i3pystatus/mail/__init__.py index afe6c25..e683f0e 100644 --- a/i3pystatus/mail/__init__.py +++ b/i3pystatus/mail/__init__.py @@ -67,7 +67,9 @@ class Mail(IntervalModule): def on_leftclick(self): if self.email_client: - run_through_shell(self.email_client, self.enable_log) + retcode, _, stderr = run_through_shell(self.email_client) + if retcode != 0 and stderr: + self.logger.error(stderr) def on_rightclick(self): self.run() diff --git a/i3pystatus/shell.py b/i3pystatus/shell.py index 12de74b..5cea607 100644 --- a/i3pystatus/shell.py +++ b/i3pystatus/shell.py @@ -1,5 +1,8 @@ from i3pystatus import IntervalModule from i3pystatus.core.command import run_through_shell +import logging + +# logger = logging.getLogger(__name__) class Shell(IntervalModule): @@ -19,8 +22,17 @@ class Shell(IntervalModule): required = ("command",) def run(self): - out, success = run_through_shell(self.command, self.enable_log, enable_shell=True) + retvalue, out, stderr = run_through_shell(self.command, enable_shell=True) + + if retvalue != 0: + self.logger.error(stderr if stderr else "Unknown error") + + if out: + out.replace("\n", " ").strip() + elif stderr: + out = stderr + self.output = { - "full_text": out, - "color": self.color if success else self.error_color + "full_text": out if out else "Command `%s` returned %d" % (self.command, retvalue), + "color": self.color if retvalue == 0 else self.error_color } diff --git a/tests/test_core_modules.py b/tests/test_core_modules.py index 362dab3..2ab34f8 100644 --- a/tests/test_core_modules.py +++ b/tests/test_core_modules.py @@ -11,7 +11,8 @@ class IntervalModuleMetaTest(unittest.TestCase): def test_no_settings(self): class NoSettings(IntervalModule): pass - self.assertTrue('interval' in NoSettings.settings) + for element in ('interval', ): + self.assertIn(element, NoSettings.settings) def test_no_interval_setting(self): class NoIntervalSetting(IntervalModule): diff --git a/tests/test_shell.py b/tests/test_shell.py new file mode 100644 index 0000000..ec9e388 --- /dev/null +++ b/tests/test_shell.py @@ -0,0 +1,24 @@ +#!/usr/bin/env python +# -*- coding: utf-8 -*- + +import unittest +import logging + +from i3pystatus.shell import Shell +from i3pystatus.core.command import run_through_shell + + +class ShellModuleMetaTest(unittest.TestCase): + + valid_output = "hello world" + + def test_shell_correct_output(self): + # ShellTest test + # http://python.readthedocs.org/en/latest/library/unittest.html + retcode, out, err = run_through_shell("echo '%s'" % (self.valid_output), enable_shell=True) + self.assertTrue(retcode == 0) + self.assertEqual(out.strip(), self.valid_output) + + def test_program_failure(self): + success, out, err = run_through_shell("thisshouldtriggeranerror") + self.assertFalse(success)