!44 start/stop command should return correct exit-code based on result of process

Merge pull request !44 from liangyongxiong/master

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__':

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."""

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):
         """

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
 

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: