Skip to content

Commit

Permalink
[SNOW-1739737] BUGFIX: Setting a default connection copies the connec…
Browse files Browse the repository at this point in the history
…tion from connections.toml to config.toml (#1791)
  • Loading branch information
sfc-gh-pjob authored Oct 25, 2024
1 parent dbaac80 commit f963b9f
Show file tree
Hide file tree
Showing 4 changed files with 96 additions and 5 deletions.
2 changes: 2 additions & 0 deletions RELEASE-NOTES.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
23 changes: 19 additions & 4 deletions src/snowflake/cli/api/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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):
Expand Down
74 changes: 74 additions & 0 deletions tests/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion tests/testing_utils/fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down

0 comments on commit f963b9f

Please sign in to comment.