From 0d8fd73d67bd19ec5d2d7f4d1f4f08787619b5a1 Mon Sep 17 00:00:00 2001 From: Jesus Bermudez Velazquez Date: Fri, 4 Aug 2023 09:36:29 +0100 Subject: [PATCH 1/8] Add logging filename, level and format Set the logging on setup_adapter and tests --- csp_billing_adapter_local/plugin.py | 15 +++++++++-- tests/data/config.yaml | 4 ++- tests/unit/test_plugin.py | 42 ++++++++++++++++++++++++++++- 3 files changed, 57 insertions(+), 4 deletions(-) diff --git a/csp_billing_adapter_local/plugin.py b/csp_billing_adapter_local/plugin.py index af65386..7e6fbe5 100644 --- a/csp_billing_adapter_local/plugin.py +++ b/csp_billing_adapter_local/plugin.py @@ -28,11 +28,13 @@ from csp_billing_adapter.config import Config -log = logging.getLogger('CSPBillingAdapter') - ADAPTER_DATA_DIR = '/var/lib/csp-billing-adapter' CACHE_FILE = 'cache.json' CSP_CONFIG_FILE = 'csp-config.json' +CSP_LOG_FILEPATH = '/var/log/csp_billing_adapter.log' +LOGGER_NAME = 'CSPBillingAdapter' + +log = logging.getLogger(LOGGER_NAME) def get_local_path(filename): @@ -44,6 +46,15 @@ def get_local_path(filename): return local_storage_path +@csp_billing_adapter.hookimpl(trylast=True) +def setup_adapter(config: Config): + log_to_file = logging.FileHandler(CSP_LOG_FILEPATH) + csp_log = logging.getLogger(LOGGER_NAME) + csp_log.addHandler(log_to_file) + logging_level = config.get('logging', {}).get('level', 'INFO') + csp_log.setLevel(logging_level) + + @csp_billing_adapter.hookimpl(trylast=True) def save_cache(config: Config, cache: dict): """Save specified content as new local cache contents.""" diff --git a/tests/data/config.yaml b/tests/data/config.yaml index 9278fec..88bf619 100644 --- a/tests/data/config.yaml +++ b/tests/data/config.yaml @@ -26,4 +26,6 @@ usage_metrics: min: 1001 min_consumption: 5 usage_aggregation: average -version: 1.1.1 \ No newline at end of file +logging: + level: 'WARN' +version: 1.1.1 diff --git a/tests/unit/test_plugin.py b/tests/unit/test_plugin.py index 636bb00..3122a11 100644 --- a/tests/unit/test_plugin.py +++ b/tests/unit/test_plugin.py @@ -22,6 +22,8 @@ test_plugin.py is part of csp-billing-adapter-local and provides units tests for the local plugin functions. """ +import logging + from pathlib import Path from tempfile import NamedTemporaryFile from unittest.mock import patch @@ -36,7 +38,8 @@ update_cache, update_csp_config, save_cache, - save_csp_config + save_csp_config, + setup_adapter ) config_file = 'tests/data/config.yaml' @@ -45,6 +48,8 @@ config_file, pm.hook ) +with open('tests/data/good/csp_config.json', 'r', encoding='utf-8') as f: + content = f.read() def test_local_get_cache(): @@ -295,3 +300,38 @@ def test_local_csp_config_save(): ) assert get_csp_config(config=local_config) == test_data2 + + +@patch('csp_billing_adapter_local.plugin.logging.getLogger') +@patch('csp_billing_adapter_local.plugin.logging.FileHandler') +def test_local_csp_setup_adapter_log_with_config_settings( + mock_logging_file_handler, mock_logging_get_logger, +): + file_handler = logging.FileHandler('foo') + log = logging.getLogger('csp_test') + mock_logging_get_logger.return_value = log + mock_logging_file_handler.return_value = file_handler + + setup_adapter(config=local_config) + + log.addHandler.assert_called_with(file_handler) + log.setLevel.assert_called_with('WARN') + mock_logging_file_handler.assert_called_with( + '/var/log/csp_billing_adapter.log' + ) + mock_logging_get_logger.assert_called_with('CSPBillingAdapter') + + +@patch('csp_billing_adapter_local.plugin.logging.Logger') +@patch('csp_billing_adapter_local.plugin.logging.getLogger') +@patch('csp_billing_adapter_local.plugin.logging.FileHandler') +def test_local_csp_setup_adapter_log_without_config_settings( + mock_logging_file_handler, mock_logging_get_logger, + mock_logger +): + log = logging.getLogger('csp_test') + + setup_adapter(config=Config({})) + + log.setLevel.assert_called_with('INFO') + From 346583ea460144f37d8ae926e98ec87a87de3630 Mon Sep 17 00:00:00 2001 From: Jesus Bermudez Velazquez Date: Mon, 7 Aug 2023 17:06:16 +0100 Subject: [PATCH 2/8] Linter issue --- tests/unit/test_plugin.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/unit/test_plugin.py b/tests/unit/test_plugin.py index 3122a11..efbdb12 100644 --- a/tests/unit/test_plugin.py +++ b/tests/unit/test_plugin.py @@ -334,4 +334,3 @@ def test_local_csp_setup_adapter_log_without_config_settings( setup_adapter(config=Config({})) log.setLevel.assert_called_with('INFO') - From 473f822f05a64626ad4748c5136cbafa57e97b3e Mon Sep 17 00:00:00 2001 From: Jesus Bermudez Velazquez Date: Mon, 7 Aug 2023 19:14:06 +0100 Subject: [PATCH 3/8] Remove unnecessary quotes --- tests/data/config.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/data/config.yaml b/tests/data/config.yaml index 88bf619..012032b 100644 --- a/tests/data/config.yaml +++ b/tests/data/config.yaml @@ -27,5 +27,5 @@ usage_metrics: min_consumption: 5 usage_aggregation: average logging: - level: 'WARN' + level: WARN version: 1.1.1 From d15482b99bfc5589d1e35fc46cee7412432d07ce Mon Sep 17 00:00:00 2001 From: Jesus Bermudez Velazquez Date: Tue, 8 Aug 2023 16:37:14 +0100 Subject: [PATCH 4/8] Set the logging level on the core code --- csp_billing_adapter_local/plugin.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/csp_billing_adapter_local/plugin.py b/csp_billing_adapter_local/plugin.py index 7e6fbe5..ef0af00 100644 --- a/csp_billing_adapter_local/plugin.py +++ b/csp_billing_adapter_local/plugin.py @@ -51,8 +51,6 @@ def setup_adapter(config: Config): log_to_file = logging.FileHandler(CSP_LOG_FILEPATH) csp_log = logging.getLogger(LOGGER_NAME) csp_log.addHandler(log_to_file) - logging_level = config.get('logging', {}).get('level', 'INFO') - csp_log.setLevel(logging_level) @csp_billing_adapter.hookimpl(trylast=True) From 187ae1825524d0d86325ab9d4afbcf3b5c1e8fb3 Mon Sep 17 00:00:00 2001 From: Jesus Bermudez Velazquez Date: Wed, 9 Aug 2023 09:52:57 +0100 Subject: [PATCH 5/8] Remove trylast for hook --- csp_billing_adapter_local/plugin.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/csp_billing_adapter_local/plugin.py b/csp_billing_adapter_local/plugin.py index ef0af00..c44b858 100644 --- a/csp_billing_adapter_local/plugin.py +++ b/csp_billing_adapter_local/plugin.py @@ -46,7 +46,7 @@ def get_local_path(filename): return local_storage_path -@csp_billing_adapter.hookimpl(trylast=True) +@csp_billing_adapter.hookimpl def setup_adapter(config: Config): log_to_file = logging.FileHandler(CSP_LOG_FILEPATH) csp_log = logging.getLogger(LOGGER_NAME) From fafef20ee7908da34cf3aaa828ece44572630e65 Mon Sep 17 00:00:00 2001 From: Jesus Bermudez Velazquez Date: Wed, 9 Aug 2023 10:34:18 +0100 Subject: [PATCH 6/8] Update tests --- tests/unit/test_plugin.py | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/tests/unit/test_plugin.py b/tests/unit/test_plugin.py index efbdb12..d3f1f1b 100644 --- a/tests/unit/test_plugin.py +++ b/tests/unit/test_plugin.py @@ -315,22 +315,7 @@ def test_local_csp_setup_adapter_log_with_config_settings( setup_adapter(config=local_config) log.addHandler.assert_called_with(file_handler) - log.setLevel.assert_called_with('WARN') mock_logging_file_handler.assert_called_with( '/var/log/csp_billing_adapter.log' ) mock_logging_get_logger.assert_called_with('CSPBillingAdapter') - - -@patch('csp_billing_adapter_local.plugin.logging.Logger') -@patch('csp_billing_adapter_local.plugin.logging.getLogger') -@patch('csp_billing_adapter_local.plugin.logging.FileHandler') -def test_local_csp_setup_adapter_log_without_config_settings( - mock_logging_file_handler, mock_logging_get_logger, - mock_logger -): - log = logging.getLogger('csp_test') - - setup_adapter(config=Config({})) - - log.setLevel.assert_called_with('INFO') From e326ffe384bfb1451d412379ea2c882d59289058 Mon Sep 17 00:00:00 2001 From: Jesus Bermudez Velazquez Date: Wed, 16 Aug 2023 10:23:35 +0100 Subject: [PATCH 7/8] Remove unused variable on test --- tests/unit/test_plugin.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/unit/test_plugin.py b/tests/unit/test_plugin.py index d3f1f1b..ba307ed 100644 --- a/tests/unit/test_plugin.py +++ b/tests/unit/test_plugin.py @@ -48,8 +48,6 @@ config_file, pm.hook ) -with open('tests/data/good/csp_config.json', 'r', encoding='utf-8') as f: - content = f.read() def test_local_get_cache(): From 64f3b1d48a0fe31a6ed4464f739d3ee4b8f4690a Mon Sep 17 00:00:00 2001 From: Jesus Bermudez Velazquez Date: Wed, 16 Aug 2023 13:48:05 +0100 Subject: [PATCH 8/8] Remove get logger call Remove get logger call in favor of using log previously declared --- csp_billing_adapter_local/plugin.py | 4 ++-- tests/unit/test_plugin.py | 13 ++++++++----- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/csp_billing_adapter_local/plugin.py b/csp_billing_adapter_local/plugin.py index c44b858..2694e6f 100644 --- a/csp_billing_adapter_local/plugin.py +++ b/csp_billing_adapter_local/plugin.py @@ -49,8 +49,8 @@ def get_local_path(filename): @csp_billing_adapter.hookimpl def setup_adapter(config: Config): log_to_file = logging.FileHandler(CSP_LOG_FILEPATH) - csp_log = logging.getLogger(LOGGER_NAME) - csp_log.addHandler(log_to_file) + log.addHandler(log_to_file) + log.info(f'Logger file handler set to {CSP_LOG_FILEPATH}') @csp_billing_adapter.hookimpl(trylast=True) diff --git a/tests/unit/test_plugin.py b/tests/unit/test_plugin.py index ba307ed..be79603 100644 --- a/tests/unit/test_plugin.py +++ b/tests/unit/test_plugin.py @@ -300,14 +300,15 @@ def test_local_csp_config_save(): assert get_csp_config(config=local_config) == test_data2 -@patch('csp_billing_adapter_local.plugin.logging.getLogger') +@patch('csp_billing_adapter_local.plugin.logging.Logger.info') +@patch('csp_billing_adapter_local.plugin.logging.Logger.addHandler') @patch('csp_billing_adapter_local.plugin.logging.FileHandler') def test_local_csp_setup_adapter_log_with_config_settings( - mock_logging_file_handler, mock_logging_get_logger, + mock_logging_file_handler, mock_logger_add_handler, + mock_logging_info ): file_handler = logging.FileHandler('foo') - log = logging.getLogger('csp_test') - mock_logging_get_logger.return_value = log + log = logging.getLogger('CSPBillingAdapter') mock_logging_file_handler.return_value = file_handler setup_adapter(config=local_config) @@ -316,4 +317,6 @@ def test_local_csp_setup_adapter_log_with_config_settings( mock_logging_file_handler.assert_called_with( '/var/log/csp_billing_adapter.log' ) - mock_logging_get_logger.assert_called_with('CSPBillingAdapter') + mock_logging_info.assert_called_with( + 'Logger file handler set to /var/log/csp_billing_adapter.log' + )