From 3302e7c405c2a90b38520ba93c7a270bcdc5ac4e Mon Sep 17 00:00:00 2001 From: Tomasz Urbaszek Date: Fri, 12 Jul 2024 10:24:10 +0200 Subject: [PATCH] fixup! Missing log directory when disabled save logs fix --- src/snowflake/cli/api/config.py | 3 +++ src/snowflake/cli/app/cli_app.py | 2 -- src/snowflake/cli/app/loggers.py | 7 ++++--- tests/test_logs.py | 17 +++++++++++------ 4 files changed, 18 insertions(+), 11 deletions(-) diff --git a/src/snowflake/cli/api/config.py b/src/snowflake/cli/api/config.py index 2a77579814..72a78a8efb 100644 --- a/src/snowflake/cli/api/config.py +++ b/src/snowflake/cli/api/config.py @@ -114,6 +114,8 @@ def config_init(config_file: Optional[Path]): Initializes the app configuration. Config provided via cli flag takes precedence. If config file does not exist we create an empty one. """ + from snowflake.cli.app.loggers import create_initial_loggers + if config_file: CONFIG_MANAGER.file_path = config_file else: @@ -121,6 +123,7 @@ def config_init(config_file: Optional[Path]): if not CONFIG_MANAGER.file_path.exists(): _initialise_config(CONFIG_MANAGER.file_path) _read_config_file() + create_initial_loggers() def add_connection(name: str, connection_config: ConnectionConfig): diff --git a/src/snowflake/cli/app/cli_app.py b/src/snowflake/cli/app/cli_app.py index 5c5272d677..e7e23f7af8 100644 --- a/src/snowflake/cli/app/cli_app.py +++ b/src/snowflake/cli/app/cli_app.py @@ -30,7 +30,6 @@ from snowflake.cli.api.output.formats import OutputFormat from snowflake.cli.api.output.types import CollectionResult from snowflake.cli.api.secure_path import SecurePath -from snowflake.cli.app import loggers from snowflake.cli.app.api_impl.plugin.plugin_config_provider_impl import ( PluginConfigProviderImpl, ) @@ -89,7 +88,6 @@ def _commands_registration_callback(value: bool): @_commands_registration.before def _config_init_callback(configuration_file: Optional[Path]): config_init(configuration_file) - loggers.create_initial_loggers() @_commands_registration.before diff --git a/src/snowflake/cli/app/loggers.py b/src/snowflake/cli/app/loggers.py index 8cd10d3856..bf63930242 100644 --- a/src/snowflake/cli/app/loggers.py +++ b/src/snowflake/cli/app/loggers.py @@ -20,9 +20,6 @@ from typing import Any, Dict, List import typer -from snowflake.cli.api.config import ( - get_logs_config, -) from snowflake.cli.api.exceptions import InvalidLogsConfiguration from snowflake.cli.api.secure_path import SecurePath from snowflake.connector.errors import ConfigSourceError @@ -103,6 +100,10 @@ def _remove_underscore_prefixes_from_keys(d: Dict[str, Any]) -> None: class FileLogsConfig: def __init__(self, debug: bool) -> None: + from snowflake.cli.api.config import ( + get_logs_config, + ) + config = get_logs_config() self.path: SecurePath = SecurePath(config["path"]) diff --git a/tests/test_logs.py b/tests/test_logs.py index a37155e5c9..92334857f8 100644 --- a/tests/test_logs.py +++ b/tests/test_logs.py @@ -69,12 +69,17 @@ def _setup_config_and_logs( ) config_path.chmod(0o700) + # Make sure we start without any leftovers + shutil.rmtree(logs_path, ignore_errors=True) clean_logging_handlers() + + # Setup loggers config_init(config_path) loggers.create_loggers(verbose=verbose, debug=debug) assert len(_list_handlers()) == (2 if save_logs else 1) + yield logs_path - shutil.rmtree(logs_path) + shutil.rmtree(logs_path, ignore_errors=True) return _setup_config_and_logs @@ -98,7 +103,7 @@ def _list_handlers(): return logging.getLogger("snowflake.cli").handlers -def _get_logs_file(logs_path: Path) -> Path: +def get_logs_file(logs_path: Path) -> Path: return next(logs_path.iterdir()) @@ -114,11 +119,11 @@ def assert_log_level(log_messages: str, expected_level: str) -> None: def assert_file_log_level(logs_path: Path, expected_level: str) -> None: - assert_log_level(_get_logs_file(logs_path).read_text(), expected_level) + assert_log_level(get_logs_file(logs_path).read_text(), expected_level) def assert_log_is_empty(logs_path: Path) -> None: - assert _get_logs_file(logs_path).read_text() == "" + assert get_logs_file(logs_path).read_text() == "" def test_logs_section_appears_in_fresh_config_file(temp_dir): @@ -145,7 +150,7 @@ def test_default_logs_location_is_created_automatically(setup_config_and_logs): def test_logs_can_be_turned_off_by_config(setup_config_and_logs): with setup_config_and_logs(save_logs=False) as logs_path: print_log_messages() - assert_log_is_empty(logs_path) + assert not logs_path.exists() def test_logs_path_is_configurable(setup_config_and_logs): @@ -209,4 +214,4 @@ def test_incorrect_log_level_in_config(setup_config_and_logs): def test_log_files_permissions(setup_config_and_logs): with setup_config_and_logs(save_logs=True) as logs_path: print_log_messages() - assert_file_permissions_are_strict(_get_logs_file(logs_path)) + assert_file_permissions_are_strict(get_logs_file(logs_path))