From f963b9fff6eb3356a4958e205d00f9f5068e037d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Job?= Date: Fri, 25 Oct 2024 15:40:31 +0200 Subject: [PATCH] [SNOW-1739737] BUGFIX: Setting a default connection copies the connection from connections.toml to config.toml (#1791) --- RELEASE-NOTES.md | 2 + src/snowflake/cli/api/config.py | 23 ++++++++-- tests/test_config.py | 74 +++++++++++++++++++++++++++++++++ tests/testing_utils/fixtures.py | 2 +- 4 files changed, 96 insertions(+), 5 deletions(-) diff --git a/RELEASE-NOTES.md b/RELEASE-NOTES.md index 0493a01f64..282c329cdb 100644 --- a/RELEASE-NOTES.md +++ b/RELEASE-NOTES.md @@ -24,6 +24,8 @@ * `snow --info` callback returns information about `SNOWFLAKE_HOME` variable. * Removed requirement of existence of any `requirements.txt` file for Python code execution via `snow git execute` command. Before the fix the file (even empty) was required to make the execution working. +* Fixed saving of the config file updates when `connections.toml` exists. + Removed incorrect copying of connections from `connections.toml` to `config.toml`. # v3.1.0 diff --git a/src/snowflake/cli/api/config.py b/src/snowflake/cli/api/config.py index cbb9db41b5..13000d33de 100644 --- a/src/snowflake/cli/api/config.py +++ b/src/snowflake/cli/api/config.py @@ -340,9 +340,21 @@ def _get_envs_for_path(*path) -> dict: } -def _dump_config(conf_file_cache: Dict): +def _dump_config(config_and_connections: Dict): + config_toml_dict = config_and_connections.copy() + + if CONNECTIONS_FILE.exists(): + # update connections in connections.toml + # it will add only connections (maybe updated) which were originally read from connections.toml + # it won't add connections from config.toml + # because config manager doesn't have connections from config.toml if connections.toml exists + _update_connections_toml(config_and_connections.get("connections") or {}) + # to config.toml save only connections from config.toml + connections_to_save_in_config_toml = _read_config_file_toml().get("connections") + config_toml_dict["connections"] = connections_to_save_in_config_toml + with SecurePath(CONFIG_MANAGER.file_path).open("w+") as fh: - dump(conf_file_cache, fh) + dump(config_toml_dict, fh) def _check_default_config_files_permissions() -> None: @@ -373,9 +385,12 @@ def _bool_or_unknown(value): return {k: _bool_or_unknown(v) for k, v in flags.items()} +def _read_config_file_toml() -> dict: + return tomlkit.loads(CONFIG_MANAGER.file_path.read_text()).unwrap() + + def _read_connections_toml() -> dict: - with open(CONNECTIONS_FILE, "r") as f: - return tomlkit.loads(f.read()).unwrap() + return tomlkit.loads(CONNECTIONS_FILE.read_text()).unwrap() def _update_connections_toml(connections: dict): diff --git a/tests/test_config.py b/tests/test_config.py index 8a9be6f4b6..f08ef2207c 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -25,6 +25,7 @@ get_connection_dict, get_default_connection_dict, get_env_variable_name, + set_config_value, ) from snowflake.cli.api.exceptions import MissingConfiguration @@ -223,6 +224,79 @@ def test_not_found_default_connection_from_evn_variable(test_root_path): ) +def test_correct_updates_of_connections_on_setting_default_connection( + test_snowcli_config, snowflake_home +): + from snowflake.cli.api.config import CONFIG_MANAGER + + connections_toml = snowflake_home / "connections.toml" + connections_toml.write_text( + """[asdf_a] + database = "asdf_a_database" + user = "asdf_a" + account = "asdf_a" + + [asdf_b] + database = "asdf_b_database" + user = "asdf_b" + account = "asdf_b" + """ + ) + config_init(test_snowcli_config) + set_config_value(section=None, key="default_connection_name", value="asdf_b") + + def assert_correct_connections_loaded(): + assert CONFIG_MANAGER["default_connection_name"] == "asdf_b" + assert CONFIG_MANAGER["connections"] == { + "asdf_a": { + "database": "asdf_a_database", + "user": "asdf_a", + "account": "asdf_a", + }, + "asdf_b": { + "database": "asdf_b_database", + "user": "asdf_b", + "account": "asdf_b", + }, + } + + # assert correct connections in memory after setting default connection name + assert_correct_connections_loaded() + + with open(connections_toml) as f: + connection_toml_content = f.read() + assert ( + connection_toml_content.count("asdf_a") == 4 + ) # connection still exists in connections.toml + assert ( + connection_toml_content.count("asdf_b") == 4 + ) # connection still exists in connections.toml + assert ( + connection_toml_content.count("jwt") == 0 + ) # connection from config.toml isn't copied to connections.toml + with open(test_snowcli_config) as f: + config_toml_content = f.read() + assert ( + config_toml_content.count("asdf_a") == 0 + ) # connection from connections.toml isn't copied to config.toml + assert ( + config_toml_content.count("asdf_b") == 1 + ) # only default_config_name setting, connection from connections.toml isn't copied to config.toml + assert ( + config_toml_content.count("connections.full") == 1 + ) # connection still exists in config.toml + assert ( + config_toml_content.count("connections.jwt") == 1 + ) # connection still exists in config.toml + assert ( + config_toml_content.count("dummy_flag = true") == 1 + ) # other settings are not erased + + # reinit config file and recheck loaded connections + config_init(test_snowcli_config) + assert_correct_connections_loaded() + + def test_connections_toml_override_config_toml(test_snowcli_config, snowflake_home): from snowflake.cli.api.config import CONFIG_MANAGER diff --git a/tests/testing_utils/fixtures.py b/tests/testing_utils/fixtures.py index 30b5fc1f63..fabed4d215 100644 --- a/tests/testing_utils/fixtures.py +++ b/tests/testing_utils/fixtures.py @@ -268,7 +268,7 @@ def temp_directory_for_app_zip(temp_dir) -> Generator: yield temp_dir.name -@pytest.fixture(scope="session") +@pytest.fixture(scope="function") def test_snowcli_config(): test_config = TEST_DIR / "test.toml" with _named_temporary_file(suffix=".toml") as p: