Skip to content

Commit

Permalink
Add feature flags usage to info and telemetry (#1042)
Browse files Browse the repository at this point in the history
  • Loading branch information
sfc-gh-turbaszek committed May 9, 2024
1 parent d08be66 commit 6e5f8b2
Show file tree
Hide file tree
Showing 9 changed files with 70 additions and 25 deletions.
1 change: 1 addition & 0 deletions RELEASE-NOTES.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
34 changes: 22 additions & 12 deletions src/snowflake/cli/api/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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()}
18 changes: 18 additions & 0 deletions src/snowflake/cli/api/utils/types.py
Original file line number Diff line number Diff line change
@@ -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()]
4 changes: 4 additions & 0 deletions src/snowflake/cli/app/cli_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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)
Expand Down
6 changes: 6 additions & 0 deletions src/snowflake/cli/app/telemetry.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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"
Expand Down Expand Up @@ -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,
}
Expand Down
12 changes: 10 additions & 2 deletions tests/app/test_telemetry.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import os
from unittest import mock

from snowflake.connector.version import VERSION as DRIVER_VERSION
Expand All @@ -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
):
Expand All @@ -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]),
Expand All @@ -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",
}
11 changes: 0 additions & 11 deletions tests/git/__snapshots__/test_git_commands.ambr
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 5 additions & 0 deletions tests/test.toml
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,8 @@ password = "dummy_password"

[connections.test_connections]
user = "python"


[cli.features]
dummy_flag = true
wrong_type_flag = "not_true"
4 changes: 4 additions & 0 deletions tests/test_main.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"},
},
]


Expand Down

0 comments on commit 6e5f8b2

Please sign in to comment.