From 58a8f12a03346a4fe38f83a8a43c241d0fe09ffe Mon Sep 17 00:00:00 2001 From: TerryHowe Date: Mon, 10 Aug 2015 12:33:28 -0600 Subject: [PATCH] Create log configuration class Configuration of logging gets triggered twice. The first time it uses the CLI options when the application is started and second it uses the configuration file after that is read. The state of the logging needs to be saved from the first to the second time, so I created a class. Implements: blueprint logging-migration Change-Id: I7b8d1a3b6fd128e98cafd7c16009c7b694a52146 --- openstackclient/shell.py | 61 ++----------------- openstackclient/tests/test_shell.py | 94 ++--------------------------- 2 files changed, 9 insertions(+), 146 deletions(-) diff --git a/openstackclient/shell.py b/openstackclient/shell.py index 6ba19d1..72f663a 100644 --- a/openstackclient/shell.py +++ b/openstackclient/shell.py @@ -94,60 +94,12 @@ class OpenStackShell(app.App): self.verify = True self.client_manager = None - - # Operation log - self.enable_operation_logging = False self.command_options = None def configure_logging(self): - """Configure logging for the app - - Cliff sets some defaults we don't want so re-work it a bit - """ - - if self.options.debug: - # --debug forces verbose_level 3 - # Set this here so cliff.app.configure_logging() can work - self.options.verbose_level = 3 - - super(OpenStackShell, self).configure_logging() - - # Set logging to the requested level - log_level = context.log_level_from_options(self.options) - context.set_warning_filter(log_level) - - # Set the handler logging level of FileHandler(--log-file) - # and StreamHandler - if self.options.log_file: - context.setup_handler_logging_level(logging.FileHandler, log_level) - - context.setup_handler_logging_level(logging.StreamHandler, log_level) - - # Requests logs some stuff at INFO that we don't want - # unless we have DEBUG - requests_log = logging.getLogger("requests") - - # Other modules we don't want DEBUG output for - cliff_log = logging.getLogger('cliff') - stevedore_log = logging.getLogger('stevedore') - iso8601_log = logging.getLogger("iso8601") - - if self.options.debug: - # --debug forces traceback - self.dump_stack_trace = True - requests_log.setLevel(logging.DEBUG) - else: - self.dump_stack_trace = False - requests_log.setLevel(logging.ERROR) - - cliff_log.setLevel(logging.ERROR) - stevedore_log.setLevel(logging.ERROR) - iso8601_log.setLevel(logging.ERROR) - - # Operation logging - self.operation_log = logging.getLogger("operation_log") - self.operation_log.setLevel(logging.ERROR) - self.operation_log.propagate = False + """Configure logging for the app.""" + self.log_configurator = context.LogConfigurator(self.options) + self.dump_stack_trace = self.log_configurator.dump_trace def run(self, argv): ret_val = 1 @@ -162,8 +114,6 @@ class OpenStackShell(app.App): self.log.error(traceback.format_exc(e)) else: self.log.error('Exception raised: ' + str(e)) - if self.enable_operation_logging: - self.operation_log.error(traceback.format_exc(e)) return ret_val @@ -287,9 +237,8 @@ class OpenStackShell(app.App): argparse=self.options, ) - # Set up every time record log in file and logging start - context.setup_logging(self, self.cloud) - + self.log_configurator.configure(self.cloud) + self.dump_stack_trace = self.log_configurator.dump_trace self.log.info("START with options: %s", self.command_options) self.log.debug("options: %s", self.options) self.log.debug("defaults: %s", cc.defaults) diff --git a/openstackclient/tests/test_shell.py b/openstackclient/tests/test_shell.py index 5b84475..cacd2fb 100644 --- a/openstackclient/tests/test_shell.py +++ b/openstackclient/tests/test_shell.py @@ -623,12 +623,9 @@ class TestShellCli(TestShell): @mock.patch("os_client_config.config.OpenStackConfig._load_vendor_file") @mock.patch("os_client_config.config.OpenStackConfig._load_config_file") - @mock.patch("openstackclient.common.context._setup_handler_for_logging") - def test_shell_args_cloud_public(self, setup_handler, config_mock, - public_mock): + def test_shell_args_cloud_public(self, config_mock, public_mock): config_mock.return_value = ('file.yaml', copy.deepcopy(CLOUD_2)) public_mock.return_value = ('file.yaml', copy.deepcopy(PUBLIC_1)) - setup_handler.return_value = mock.MagicMock() _shell = make_shell() fake_execute( @@ -666,12 +663,9 @@ class TestShellCli(TestShell): @mock.patch("os_client_config.config.OpenStackConfig._load_vendor_file") @mock.patch("os_client_config.config.OpenStackConfig._load_config_file") - @mock.patch("openstackclient.common.context._setup_handler_for_logging") - def test_shell_args_precedence(self, setup_handler, config_mock, - vendor_mock): + def test_shell_args_precedence(self, config_mock, vendor_mock): config_mock.return_value = ('file.yaml', copy.deepcopy(CLOUD_2)) vendor_mock.return_value = ('file.yaml', copy.deepcopy(PUBLIC_1)) - setup_handler.return_value = mock.MagicMock() _shell = make_shell() # Test command option overriding config file value @@ -723,12 +717,9 @@ class TestShellCliEnv(TestShell): @mock.patch("os_client_config.config.OpenStackConfig._load_vendor_file") @mock.patch("os_client_config.config.OpenStackConfig._load_config_file") - @mock.patch("openstackclient.common.context._setup_handler_for_logging") - def test_shell_args_precedence_1(self, setup_handler, config_mock, - vendor_mock): + def test_shell_args_precedence_1(self, config_mock, vendor_mock): config_mock.return_value = ('file.yaml', copy.deepcopy(CLOUD_2)) vendor_mock.return_value = ('file.yaml', copy.deepcopy(PUBLIC_1)) - setup_handler.return_value = mock.MagicMock() _shell = make_shell() # Test env var @@ -767,12 +758,9 @@ class TestShellCliEnv(TestShell): @mock.patch("os_client_config.config.OpenStackConfig._load_vendor_file") @mock.patch("os_client_config.config.OpenStackConfig._load_config_file") - @mock.patch("openstackclient.common.context._setup_handler_for_logging") - def test_shell_args_precedence_2(self, setup_handler, config_mock, - vendor_mock): + def test_shell_args_precedence_2(self, config_mock, vendor_mock): config_mock.return_value = ('file.yaml', copy.deepcopy(CLOUD_2)) vendor_mock.return_value = ('file.yaml', copy.deepcopy(PUBLIC_1)) - setup_handler.return_value = mock.MagicMock() _shell = make_shell() # Test command option overriding config file value @@ -810,77 +798,3 @@ class TestShellCliEnv(TestShell): 'krikkit', _shell.cloud.config['region_name'], ) - - -class TestShellCliLogging(TestShell): - def setUp(self): - super(TestShellCliLogging, self).setUp() - - def tearDown(self): - super(TestShellCliLogging, self).tearDown() - - @mock.patch("os_client_config.config.OpenStackConfig._load_vendor_file") - @mock.patch("os_client_config.config.OpenStackConfig._load_config_file") - @mock.patch("openstackclient.common.context._setup_handler_for_logging") - def test_shell_args_precedence_1(self, setup_handler, config_mock, - vendor_mock): - config_mock.return_value = ('file.yaml', copy.deepcopy(CLOUD_2)) - vendor_mock.return_value = ('file.yaml', copy.deepcopy(PUBLIC_1)) - setup_handler.return_value = mock.MagicMock() - _shell = make_shell() - - # These come from clouds.yaml - fake_execute( - _shell, - "--os-cloud megacloud list user", - ) - self.assertEqual( - 'megacloud', - _shell.cloud.name, - ) - - self.assertEqual( - '/tmp/test_log_file', - _shell.cloud.config['log_file'], - ) - self.assertEqual( - 'debug', - _shell.cloud.config['log_level'], - ) - - @mock.patch("os_client_config.config.OpenStackConfig._load_vendor_file") - @mock.patch("os_client_config.config.OpenStackConfig._load_config_file") - def test_shell_args_precedence_2(self, config_mock, vendor_mock): - config_mock.return_value = ('file.yaml', copy.deepcopy(CLOUD_1)) - vendor_mock.return_value = ('file.yaml', copy.deepcopy(PUBLIC_1)) - _shell = make_shell() - - # Test operation_log_file not set - fake_execute( - _shell, - "--os-cloud scc list user", - ) - self.assertEqual( - False, - _shell.enable_operation_logging, - ) - - @mock.patch("os_client_config.config.OpenStackConfig._load_vendor_file") - @mock.patch("os_client_config.config.OpenStackConfig._load_config_file") - @mock.patch("openstackclient.common.context._setup_handler_for_logging") - def test_shell_args_precedence_3(self, setup_handler, config_mock, - vendor_mock): - config_mock.return_value = ('file.yaml', copy.deepcopy(CLOUD_2)) - vendor_mock.return_value = ('file.yaml', copy.deepcopy(PUBLIC_1)) - setup_handler.return_value = mock.MagicMock() - _shell = make_shell() - - # Test enable_operation_logging set - fake_execute( - _shell, - "--os-cloud megacloud list user", - ) - self.assertEqual( - True, - _shell.enable_operation_logging, - )