From 6e5f8b2b4463b4ee2c9664533c0e87353bd20cbc Mon Sep 17 00:00:00 2001 From: Tomasz Urbaszek Date: Thu, 9 May 2024 16:51:15 +0200 Subject: [PATCH] Add feature flags usage to info and telemetry (#1042) --- RELEASE-NOTES.md | 1 + src/snowflake/cli/api/config.py | 34 ++++++++++++------- src/snowflake/cli/api/utils/types.py | 18 ++++++++++ src/snowflake/cli/app/cli_app.py | 4 +++ src/snowflake/cli/app/telemetry.py | 6 ++++ tests/app/test_telemetry.py | 12 +++++-- .../git/__snapshots__/test_git_commands.ambr | 11 ------ tests/test.toml | 5 +++ tests/test_main.py | 4 +++ 9 files changed, 70 insertions(+), 25 deletions(-) create mode 100644 src/snowflake/cli/api/utils/types.py diff --git a/RELEASE-NOTES.md b/RELEASE-NOTES.md index 1b4ec380a..7c1288a2c 100644 --- a/RELEASE-NOTES.md +++ b/RELEASE-NOTES.md @@ -19,6 +19,7 @@ * Added support for fully qualified stage names in stage and git execute commands. * Fixed a bug where `snow app run` was not upgrading the application when the local state and remote stage are identical (for example immediately after `snow app deploy`). * Fixed handling of stage path separators on Windows +* The `--info` callback returns info about configured feature flags. # v2.2.0 diff --git a/src/snowflake/cli/api/config.py b/src/snowflake/cli/api/config.py index fa3ae832c..35bbd0852 100644 --- a/src/snowflake/cli/api/config.py +++ b/src/snowflake/cli/api/config.py @@ -17,6 +17,7 @@ ) from snowflake.cli.api.secure_path import SecurePath from snowflake.cli.api.secure_utils import file_permissions_are_strict +from snowflake.cli.api.utils.types import try_cast_to_bool from snowflake.connector.compat import IS_WINDOWS from snowflake.connector.config_manager import CONFIG_MANAGER from snowflake.connector.constants import CONFIG_FILE, CONNECTIONS_FILE @@ -245,21 +246,12 @@ def get_config_value(*path, key: str, default: Optional[Any] = Empty) -> Any: def get_config_bool_value(*path, key: str, default: Optional[Any] = Empty) -> bool: value = get_config_value(*path, key=key, default=default) - # If we get bool then we can return - if isinstance(value, bool): - return value - - # Now if value is not string then cast it to str. Simplifies logic for 1 and 0 - if not isinstance(value, str): - value = str(value) - - know_booleans_mapping = {"true": True, "false": False, "1": True, "0": False} - - if value.lower() not in know_booleans_mapping: + try: + return try_cast_to_bool(value) + except ValueError: raise ClickException( f"Expected boolean value for {'.'.join((*path, key))} option." ) - return know_booleans_mapping[value.lower()] def _initialise_config(config_file: Path) -> None: @@ -318,3 +310,21 @@ def _check_default_config_files_permissions() -> None: raise ConfigFileTooWidePermissionsError(CONNECTIONS_FILE) if CONFIG_FILE.exists() and not file_permissions_are_strict(CONFIG_FILE): raise ConfigFileTooWidePermissionsError(CONFIG_FILE) + + +from typing import Literal + + +def get_feature_flags_section() -> Dict[str, bool | Literal["UNKNOWN"]]: + if not config_section_exists(*FEATURE_FLAGS_SECTION_PATH): + return {} + + flags = get_config_section(*FEATURE_FLAGS_SECTION_PATH) + + def _bool_or_unknown(value): + try: + return try_cast_to_bool(value) + except ValueError: + return "UNKNOWN" + + return {k: _bool_or_unknown(v) for k, v in flags.items()} diff --git a/src/snowflake/cli/api/utils/types.py b/src/snowflake/cli/api/utils/types.py new file mode 100644 index 000000000..4e4cc2e21 --- /dev/null +++ b/src/snowflake/cli/api/utils/types.py @@ -0,0 +1,18 @@ +from __future__ import annotations + +from typing import Any + + +def try_cast_to_bool(value: Any) -> bool: + if isinstance(value, bool): + return value + + # Now if value is not string then cast it to str. Simplifies logic for 1 and 0 + if not isinstance(value, str): + value = str(value) + + know_booleans_mapping = {"true": True, "false": False, "1": True, "0": False} + + if value.lower() not in know_booleans_mapping: + raise ValueError(f"Could not case {value} to bool value") + return know_booleans_mapping[value.lower()] diff --git a/src/snowflake/cli/app/cli_app.py b/src/snowflake/cli/app/cli_app.py index a7e22e08e..077738be2 100644 --- a/src/snowflake/cli/app/cli_app.py +++ b/src/snowflake/cli/app/cli_app.py @@ -107,6 +107,9 @@ def _version_callback(value: bool): _exit_with_cleanup() +from snowflake.cli.api.config import get_feature_flags_section + + @_do_not_execute_on_completion def _info_callback(value: bool): if value: @@ -119,6 +122,7 @@ def _info_callback(value: bool): }, {"key": "python_version", "value": sys.version}, {"key": "system_info", "value": platform.platform()}, + {"key": "feature_flags", "value": get_feature_flags_section()}, ], ) print_result(result, output_format=OutputFormat.JSON) diff --git a/src/snowflake/cli/app/telemetry.py b/src/snowflake/cli/app/telemetry.py index 63e987b1d..c1e2e4e52 100644 --- a/src/snowflake/cli/app/telemetry.py +++ b/src/snowflake/cli/app/telemetry.py @@ -8,6 +8,7 @@ import click from snowflake.cli.__about__ import VERSION from snowflake.cli.api.cli_global_context import cli_context +from snowflake.cli.api.config import get_feature_flags_section from snowflake.cli.api.output.formats import OutputFormat from snowflake.cli.api.utils.error_handling import ignore_exceptions from snowflake.cli.app.constants import PARAM_APPLICATION_NAME @@ -30,6 +31,8 @@ class CLITelemetryField(Enum): COMMAND_GROUP = "command_group" COMMAND_FLAGS = "command_flags" COMMAND_OUTPUT_TYPE = "command_output_type" + # Configuration + CONFIG_FEATURE_FLAGS = "config_feature_flags" # Information EVENT = "event" ERROR_MSG = "error_msg" @@ -84,6 +87,9 @@ def generate_telemetry_data_dict( CLITelemetryField.VERSION_CLI: VERSION, CLITelemetryField.VERSION_OS: platform.platform(), CLITelemetryField.VERSION_PYTHON: python_version(), + CLITelemetryField.CONFIG_FEATURE_FLAGS: { + k: str(v) for k, v in get_feature_flags_section().items() + }, **_find_command_info(), **telemetry_payload, } diff --git a/tests/app/test_telemetry.py b/tests/app/test_telemetry.py index fc10be563..1594f2651 100644 --- a/tests/app/test_telemetry.py +++ b/tests/app/test_telemetry.py @@ -1,3 +1,4 @@ +import os from unittest import mock from snowflake.connector.version import VERSION as DRIVER_VERSION @@ -10,6 +11,7 @@ @mock.patch("snowflake.cli.app.telemetry.get_time_millis") @mock.patch("snowflake.connector.connect") @mock.patch("snowflake.cli.plugins.connection.commands.ObjectManager") +@mock.patch.dict(os.environ, {"SNOWFLAKE_CLI_FEATURES_FOO": "False"}) def test_executing_command_sends_telemetry_data( _, mock_conn, mock_time, mock_platform, mock_version, runner ): @@ -21,9 +23,10 @@ def test_executing_command_sends_telemetry_data( assert result.exit_code == 0, result.output # The method is called with a TelemetryData type, so we cast it to dict for simpler comparison - assert mock_conn.return_value._telemetry.try_add_log_to_batch.call_args.args[ # noqa: SLF001 + actual_call = mock_conn.return_value._telemetry.try_add_log_to_batch.call_args.args[ # noqa: SLF001 0 - ].to_dict() == { + ].to_dict() + assert actual_call == { "message": { "driver_type": "PythonConnector", "driver_version": ".".join(str(s) for s in DRIVER_VERSION[:3]), @@ -36,6 +39,11 @@ def test_executing_command_sends_telemetry_data( "command_flags": {"diag_log_path": "DEFAULT", "format": "DEFAULT"}, "command_output_type": "TABLE", "type": "executing_command", + "config_feature_flags": { + "dummy_flag": "True", + "foo": "False", + "wrong_type_flag": "UNKNOWN", + }, }, "timestamp": "123", } diff --git a/tests/git/__snapshots__/test_git_commands.ambr b/tests/git/__snapshots__/test_git_commands.ambr index 01b771909..0f6c6b8b5 100644 --- a/tests/git/__snapshots__/test_git_commands.ambr +++ b/tests/git/__snapshots__/test_git_commands.ambr @@ -12,17 +12,6 @@ ''' # --- -# name: test_execute[@db.schema.repo/branches/main/s1.sql-@db.schema.repo/branches/main/-expected_files0] - ''' - SUCCESS - @db.schema.repo/branches/main/s1.sql - +--------------------------------------------------------+ - | File | Status | Error | - |--------------------------------------+---------+-------| - | @db.schema.repo/branches/main/s1.sql | SUCCESS | None | - +--------------------------------------------------------+ - - ''' -# --- # name: test_execute[@db.schema.repo/branches/main/s1.sql-@db.schema.repo/branches/main/-expected_files3] ''' SUCCESS - @db.schema.repo/branches/main/s1.sql diff --git a/tests/test.toml b/tests/test.toml index 4c9a34bf7..8a89d8324 100644 --- a/tests/test.toml +++ b/tests/test.toml @@ -21,3 +21,8 @@ password = "dummy_password" [connections.test_connections] user = "python" + + +[cli.features] +dummy_flag = true +wrong_type_flag = "not_true" diff --git a/tests/test_main.py b/tests/test_main.py index 8018a4d62..cd351a865 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -63,6 +63,10 @@ def test_info_callback(runner): {"key": "default_config_file_path", "value": str(CONFIG_MANAGER.file_path)}, {"key": "python_version", "value": sys.version}, {"key": "system_info", "value": platform.platform()}, + { + "key": "feature_flags", + "value": {"dummy_flag": True, "wrong_type_flag": "UNKNOWN"}, + }, ]