From 1bc356ad11c2ccef07578fdd8d26f75357a73ca0 Mon Sep 17 00:00:00 2001 From: liangyongxiong Date: Mon, 20 Apr 2020 10:17:33 +0800 Subject: [PATCH] 1) fix return code for start/stop command; 2) replace print with standard console logger --- mindinsight/backend/run.py | 8 +++++--- mindinsight/scripts/start.py | 11 ++++++----- mindinsight/scripts/stop.py | 15 ++++++++------- mindinsight/utils/command.py | 27 +++++++++++++++++++-------- mindinsight/utils/log.py | 6 +++--- 5 files changed, 41 insertions(+), 26 deletions(-) diff --git a/mindinsight/backend/run.py b/mindinsight/backend/run.py index 2bd40d51..2036f0da 100644 --- a/mindinsight/backend/run.py +++ b/mindinsight/backend/run.py @@ -231,6 +231,8 @@ def start(): log_size = _get_file_size(errorlog_abspath) + stdout = setup_logger('mindinsight', 'stdout', console=True, logfile=False, formatter='%(message)s') + # start server process = subprocess.Popen( shlex.split(cmd), @@ -241,15 +243,15 @@ def start(): ) _, stderr = process.communicate() if stderr: - print(stderr.decode()) + stdout.error(stderr.decode()) # wait command success to end when gunicorn running in daemon. if gunicorn_conf.daemon and process.wait() == 0: state_result = _check_server_start_stat(errorlog_abspath, log_size) # print gunicorn start state to stdout - print('Web address: http://{}:{}'.format(settings.HOST, settings.PORT)) + stdout.info('Web address: http://{}:{}'.format(settings.HOST, settings.PORT)) for line in state_result["prompt_message"]: - print(line) + stdout.info(line) if __name__ == '__main__': diff --git a/mindinsight/scripts/start.py b/mindinsight/scripts/start.py index 3ce62b83..96b94f42 100644 --- a/mindinsight/scripts/start.py +++ b/mindinsight/scripts/start.py @@ -15,6 +15,7 @@ """Start mindinsight service.""" import os +import sys import argparse from importlib import import_module @@ -183,19 +184,19 @@ class Command(BaseCommand): """ for key, value in args.__dict__.items(): if value is not None: - self.logger.info('%s = %s', key, value) + self.logfile.info('%s = %s', key, value) try: self.check_port() except PortNotAvailableError as error: - print(error.message) - self.logger.error(error.message) - return + self.console.error(error.message) + self.logfile.error(error.message) + sys.exit(1) run_module = import_module('mindinsight.backend.run') run_module.start() - self.logger.info('Start mindinsight done.') + self.logfile.info('Start mindinsight done.') def check_port(self): """Check port.""" diff --git a/mindinsight/scripts/stop.py b/mindinsight/scripts/stop.py index 90c3832e..38867aff 100644 --- a/mindinsight/scripts/stop.py +++ b/mindinsight/scripts/stop.py @@ -15,6 +15,7 @@ """Stop mindinsight service.""" import os +import sys import argparse import signal import getpass @@ -96,10 +97,10 @@ class Command(BaseCommand): port, pid = args.port, args.pid if not pid: msg = f'No mindinsight service found for port {port}' - print(msg) - return + self.console.error(msg) + sys.exit(1) - self.logger.info('Stop mindinsight with port %s and pid %s.', port, pid) + self.logfile.info('Stop mindinsight with port %s and pid %s.', port, pid) process = psutil.Process(pid) child_pids = [child.pid for child in process.children()] @@ -108,8 +109,8 @@ class Command(BaseCommand): try: os.kill(pid, signal.SIGKILL) except PermissionError: - print('kill pid %s failed due to permission error' % pid) - return + self.console.info('kill pid %s failed due to permission error' % pid) + sys.exit(1) # cleanup gunicorn worker processes for child_pid in child_pids: @@ -119,9 +120,9 @@ class Command(BaseCommand): pass for hook in HookUtils.instance().hooks(): - hook.on_shutdown(self.logger) + hook.on_shutdown(self.logfile) - print('Stop mindinsight service successfully') + self.console.info('Stop mindinsight service successfully') def get_process(self, port): """ diff --git a/mindinsight/utils/command.py b/mindinsight/utils/command.py index e47f0558..c4680831 100644 --- a/mindinsight/utils/command.py +++ b/mindinsight/utils/command.py @@ -30,7 +30,11 @@ class BaseCommand: name = '' description = '' - logger = None + # logger for console output instead of built-in print + console = None + + # logger for log file recording in case audit is required + logfile = None def add_arguments(self, parser): """ @@ -64,21 +68,28 @@ class BaseCommand: Args: args (Namespace): parsed arguments to hold customized parameters. """ + error = None try: self.update_settings(args) - except MindInsightException as error: - print(error.message) - else: - self.logger = setup_logger('scripts', self.name) - self.run(args) + except MindInsightException as e: + error = e + + self.console = setup_logger('mindinsight', 'console', console=True, logfile=False, formatter='%(message)s') + if error is not None: + self.console.error(error.message) + sys.exit(1) + + self.logfile = setup_logger('scripts', self.name, console=False, logfile=True) + self.run(args) def main(): """Entry point for mindinsight CLI.""" + console = setup_logger('mindinsight', 'console', console=True, logfile=False, formatter='%(message)s') if (sys.version_info.major, sys.version_info.minor) < (3, 7): - print('Python version should be at least 3.7') - return + console.error('Python version should be at least 3.7') + sys.exit(1) permissions = os.R_OK | os.W_OK | os.X_OK diff --git a/mindinsight/utils/log.py b/mindinsight/utils/log.py index 90b3f409..d77ab980 100644 --- a/mindinsight/utils/log.py +++ b/mindinsight/utils/log.py @@ -146,7 +146,7 @@ def get_logger(sub_module, log_name): return logging.getLogger(name='{}.{}'.format(sub_module, log_name)) -def setup_logger(sub_module, log_name, console=False, logfile=True, **kwargs): +def setup_logger(sub_module, log_name, **kwargs): """ Setup logger with sub module name and log file name. @@ -189,12 +189,12 @@ def setup_logger(sub_module, log_name, console=False, logfile=True, **kwargs): if not formatter: formatter = settings.LOG_FORMAT - if console: + if kwargs.get('console', True): console_handler = logging.StreamHandler(sys.stdout) console_handler.formatter = MindInsightFormatter(sub_module, formatter) logger.addHandler(console_handler) - if logfile: + if kwargs.get('logfile', True): max_bytes = kwargs.get('maxBytes', settings.LOG_ROTATING_MAXBYTES) if not isinstance(max_bytes, int) or not max_bytes > 0: