diff --git a/src/snowflake/cli/_plugins/connection/util.py b/src/snowflake/cli/_plugins/connection/util.py index 5a317b50b5..45c6873e0a 100644 --- a/src/snowflake/cli/_plugins/connection/util.py +++ b/src/snowflake/cli/_plugins/connection/util.py @@ -19,7 +19,6 @@ import os from enum import Enum from functools import lru_cache -from textwrap import dedent from typing import Any, Dict, Optional from click.exceptions import ClickException @@ -57,11 +56,12 @@ class UIParameter(Enum): NA_ENFORCE_MANDATORY_FILTERS = ( "ENFORCE_MANDATORY_FILTERS_FOR_SAME_ACCOUNT_INSTALLATION" ) + NA_FEATURE_RELEASE_CHANNELS = "FEATURE_RELEASE_CHANNELS" def get_ui_parameter( conn: SnowflakeConnection, parameter: UIParameter, default: Any -) -> str: +) -> Any: """ Returns the value of a single UI parameter. If the parameter is not found, the default value is returned. @@ -77,21 +77,19 @@ def get_ui_parameters(conn: SnowflakeConnection) -> Dict[UIParameter, Any]: Returns the UI parameters from the SYSTEM$BOOTSTRAP_DATA_REQUEST function """ - parameters_to_fetch = sorted([param.value for param in UIParameter]) + parameters_to_fetch = [param.value for param in UIParameter] - query = dedent( - f""" - select value['value']::string as PARAM_VALUE, value['name']::string as PARAM_NAME from table(flatten( - input => parse_json(SYSTEM$BOOTSTRAP_DATA_REQUEST()), - path => 'clientParamsInfo' - )) where value['name'] in ('{"', '".join(parameters_to_fetch)}'); - """ - ) + # Parsing of the Json and the filtering is happening here in Snowflake CLI + # in order to avoid requiring a warehouse in Snowflake + query = "call system$bootstrap_data_request('CLIENT_PARAMS_INFO')" + *_, cursor = conn.execute_string(query) - *_, cursor = conn.execute_string(query, cursor_class=DictCursor) + json_map = json.loads(cursor.fetchone()[0]) return { - UIParameter(row["PARAM_NAME"]): row["PARAM_VALUE"] for row in cursor.fetchall() + UIParameter(row["name"]): row["value"] + for row in json_map["clientParamsInfo"] + if row["name"] in parameters_to_fetch } @@ -103,12 +101,7 @@ def is_regionless_redirect(conn: SnowflakeConnection) -> bool: assume it's regionless, as this is true for most production deployments. """ try: - return ( - get_ui_parameter( - conn, UIParameter.NA_ENABLE_REGIONLESS_REDIRECT, "true" - ).lower() - == "true" - ) + return get_ui_parameter(conn, UIParameter.NA_ENABLE_REGIONLESS_REDIRECT, True) except: log.warning( "Cannot determine regionless redirect; assuming True.", exc_info=True diff --git a/src/snowflake/cli/_plugins/nativeapp/entities/application.py b/src/snowflake/cli/_plugins/nativeapp/entities/application.py index 1f794c2db9..5057f86c60 100644 --- a/src/snowflake/cli/_plugins/nativeapp/entities/application.py +++ b/src/snowflake/cli/_plugins/nativeapp/entities/application.py @@ -13,7 +13,6 @@ from pydantic import Field, field_validator from snowflake.cli._plugins.connection.util import ( UIParameter, - get_ui_parameter, make_snowsight_url, ) from snowflake.cli._plugins.nativeapp.artifacts import ( @@ -124,18 +123,12 @@ def __init__( self._is_dev_mode = install_method.is_dev_mode self._metrics = get_cli_context().metrics self._console = console - connection = get_sql_executor()._conn # noqa: SLF001 - self._event_sharing_enabled = ( - get_ui_parameter( - connection, UIParameter.NA_EVENT_SHARING_V2, "true" - ).lower() - == "true" + + self._event_sharing_enabled = get_snowflake_facade().get_ui_parameter( + UIParameter.NA_EVENT_SHARING_V2, True ) - self._event_sharing_enforced = ( - get_ui_parameter( - connection, UIParameter.NA_ENFORCE_MANDATORY_FILTERS, "true" - ).lower() - == "true" + self._event_sharing_enforced = get_snowflake_facade().get_ui_parameter( + UIParameter.NA_ENFORCE_MANDATORY_FILTERS, True ) self._share_mandatory_events = ( diff --git a/src/snowflake/cli/_plugins/nativeapp/entities/application_package.py b/src/snowflake/cli/_plugins/nativeapp/entities/application_package.py index 83f32d8b52..c61b7939dc 100644 --- a/src/snowflake/cli/_plugins/nativeapp/entities/application_package.py +++ b/src/snowflake/cli/_plugins/nativeapp/entities/application_package.py @@ -9,6 +9,7 @@ import typer from click import BadOptionUsage, ClickException from pydantic import Field, field_validator +from snowflake.cli._plugins.connection.util import UIParameter from snowflake.cli._plugins.nativeapp.artifacts import ( BundleMap, VersionInfo, @@ -25,7 +26,6 @@ NAME_COL, OWNER_COL, PATCH_COL, - SPECIAL_COMMENT, VERSION_COL, ) from snowflake.cli._plugins.nativeapp.exceptions import ( @@ -35,6 +35,7 @@ ObjectPropertyNotFoundError, SetupScriptFailedValidation, ) +from snowflake.cli._plugins.nativeapp.feature_flags import FeatureFlag from snowflake.cli._plugins.nativeapp.policy import ( AllowAlwaysPolicy, AskAlwaysPolicy, @@ -824,6 +825,28 @@ def verify_project_distribution( return False return True + def _get_enable_release_channels_flag(self) -> Optional[bool]: + """ + Returns the requested value of enable_release_channels flag for the application package. + It retrieves the value from the configuration file and checks that the feature is enabled in the account. + If return value is None, it means do not explicitly set the flag. + """ + feature_flag_from_config = FeatureFlag.ENABLE_RELEASE_CHANNELS.get_value() + feature_enabled_in_account = ( + get_snowflake_facade().get_ui_parameter( + UIParameter.NA_FEATURE_RELEASE_CHANNELS, "ENABLED" + ) + == "ENABLED" + ) + + if feature_flag_from_config is not None and not feature_enabled_in_account: + self._workspace_ctx.console.warning( + f"Ignoring feature flag {FeatureFlag.ENABLE_RELEASE_CHANNELS.name} because release channels are not enabled in the current account." + ) + return None + + return feature_flag_from_config + def create_app_package(self) -> None: """ Creates the application package with our up-to-date stage if none exists. @@ -851,21 +874,23 @@ def create_app_package(self) -> None: if row_comment not in ALLOWED_SPECIAL_COMMENTS: raise ApplicationPackageAlreadyExistsError(self.name) + # 4. Update the application package with setting enable_release_channels if necessary + get_snowflake_facade().alter_application_package_properties( + package_name=self.name, + enable_release_channels=self._get_enable_release_channels_flag(), + role=self.role, + ) + return # If no application package pre-exists, create an application package, with the specified distribution in the project definition file. - sql_executor = get_sql_executor() - with sql_executor.use_role(self.role): - console.step(f"Creating new application package {self.name} in account.") - sql_executor.execute_query( - dedent( - f"""\ - create application package {self.name} - comment = {SPECIAL_COMMENT} - distribution = {model.distribution} - """ - ) - ) + console.step(f"Creating new application package {self.name} in account.") + get_snowflake_facade().create_application_package( + role=self.role, + enable_release_channels=self._get_enable_release_channels_flag(), + distribution=model.distribution, + package_name=self.name, + ) def execute_post_deploy_hooks(self): execute_post_deploy_hooks( diff --git a/src/snowflake/cli/_plugins/nativeapp/feature_flags.py b/src/snowflake/cli/_plugins/nativeapp/feature_flags.py index 14143619ea..dbc47e7483 100644 --- a/src/snowflake/cli/_plugins/nativeapp/feature_flags.py +++ b/src/snowflake/cli/_plugins/nativeapp/feature_flags.py @@ -22,3 +22,4 @@ class FeatureFlag(FeatureFlagMixin): ENABLE_NATIVE_APP_PYTHON_SETUP = BooleanFlag( "ENABLE_NATIVE_APP_PYTHON_SETUP", False ) + ENABLE_RELEASE_CHANNELS = BooleanFlag("ENABLE_RELEASE_CHANNELS", None) diff --git a/src/snowflake/cli/_plugins/nativeapp/sf_sql_facade.py b/src/snowflake/cli/_plugins/nativeapp/sf_sql_facade.py index 569898ed2d..e0fdbdb284 100644 --- a/src/snowflake/cli/_plugins/nativeapp/sf_sql_facade.py +++ b/src/snowflake/cli/_plugins/nativeapp/sf_sql_facade.py @@ -18,6 +18,7 @@ from textwrap import dedent from typing import Any, Dict, List +from snowflake.cli._plugins.connection.util import UIParameter, get_ui_parameter from snowflake.cli._plugins.nativeapp.constants import ( AUTHORIZE_TELEMETRY_COL, NAME_COL, @@ -49,6 +50,7 @@ ) from snowflake.cli.api.identifiers import FQN from snowflake.cli.api.metrics import CLICounterField +from snowflake.cli.api.project.schemas.v1.native_app.package import DistributionOptions from snowflake.cli.api.project.util import ( identifier_to_show_like_pattern, is_valid_unquoted_identifier, @@ -743,6 +745,100 @@ def create_application( return create_cursor.fetchall() + def create_application_package( + self, + package_name: str, + distribution: DistributionOptions, + enable_release_channels: bool | None = None, + role: str | None = None, + ) -> None: + """ + Creates a new application package. + @param package_name: Name of the application package to create. + @param [Optional] enable_release_channels: Enable/Disable release channels if not None. + @param [Optional] role: Role to switch to while running this script. Current role will be used if no role is passed in. + """ + package_name = to_identifier(package_name) + + enable_release_channels_clause = "" + if enable_release_channels is not None: + enable_release_channels_clause = ( + f"enable_release_channels = {str(enable_release_channels).lower()}" + ) + + with self._use_role_optional(role): + try: + self._sql_executor.execute_query( + dedent( + _strip_empty_lines( + f"""\ + create application package {package_name} + comment = {SPECIAL_COMMENT} + distribution = {distribution} + {enable_release_channels_clause} + """ + ) + ) + ) + except ProgrammingError as err: + if err.errno == INSUFFICIENT_PRIVILEGES: + raise InsufficientPrivilegesError( + f"Insufficient privileges to create application package {package_name}", + role=role, + ) from err + handle_unclassified_error( + err, f"Failed to create application package {package_name}." + ) + + def alter_application_package_properties( + self, + package_name: str, + enable_release_channels: bool | None = None, + role: str | None = None, + ) -> None: + """ + Alters the properties of an existing application package. + @param package_name: Name of the application package to alter. + @param [Optional] enable_release_channels: Enable/Disable release channels if not None. + @param [Optional] role: Role to switch to while running this script. Current role will be used if no role is passed in. + """ + + package_name = to_identifier(package_name) + + if enable_release_channels is not None: + with self._use_role_optional(role): + try: + self._sql_executor.execute_query( + dedent( + f"""\ + alter application package {package_name} + set enable_release_channels = {str(enable_release_channels).lower()} + """ + ) + ) + except ProgrammingError as err: + if err.errno == INSUFFICIENT_PRIVILEGES: + raise InsufficientPrivilegesError( + f"Insufficient privileges update enable_release_channels for application package {package_name}", + role=role, + ) from err + handle_unclassified_error( + err, + f"Failed to update enable_release_channels for application package {package_name}.", + ) + + def get_ui_parameter(self, parameter: UIParameter, default: Any) -> Any: + """ + Returns the value of a single UI parameter. + If the parameter is not found, the default value is returned. + + @param parameter: UIParameter, the parameter to get the value of. + @param default: Default value to return if the parameter is not found. + """ + connection = self._sql_executor._conn # noqa SLF001 + + return get_ui_parameter(connection, parameter, default) + # TODO move this to src/snowflake/cli/api/project/util.py in a separate # PR since it's codeowned by the CLI team @@ -763,3 +859,10 @@ def _same_identifier(id1: str, id2: str) -> bool: # The canonical identifiers are equal if they are equal when both are quoted # (if they are already quoted, this is a no-op) return to_quoted_identifier(canonical_id1) == to_quoted_identifier(canonical_id2) + + +def _strip_empty_lines(text: str) -> str: + """ + Strips empty lines from the input string. + """ + return "\n".join(line for line in text.splitlines() if line.strip()) diff --git a/src/snowflake/cli/api/config.py b/src/snowflake/cli/api/config.py index b6e26a68d7..0a03c36b97 100644 --- a/src/snowflake/cli/api/config.py +++ b/src/snowflake/cli/api/config.py @@ -286,8 +286,12 @@ def get_config_value(*path, key: str, default: Optional[Any] = Empty) -> Any: raise -def get_config_bool_value(*path, key: str, default: Optional[Any] = Empty) -> bool: - value = get_config_value(*path, key=key, default=default) +def get_config_bool_value(*path, key: str, default: Optional[bool]) -> Optional[bool]: + value = get_config_value(*path, key=key, default=None) + + if value is None: + return default + try: return try_cast_to_bool(value) except ValueError: diff --git a/src/snowflake/cli/api/feature_flags.py b/src/snowflake/cli/api/feature_flags.py index 2ed9728e55..d504056e02 100644 --- a/src/snowflake/cli/api/feature_flags.py +++ b/src/snowflake/cli/api/feature_flags.py @@ -24,20 +24,31 @@ class BooleanFlag(NamedTuple): name: str - default: bool = False + default: bool | None = False @unique class FeatureFlagMixin(Enum): - def is_enabled(self) -> bool: + def get_value(self) -> bool | None: return get_config_bool_value( *FEATURE_FLAGS_SECTION_PATH, key=self.value.name.lower(), default=self.value.default, ) - def is_disabled(self): - return not self.is_enabled() + def is_enabled(self) -> bool: + return self.get_value() is True + + def is_disabled(self) -> bool: + return self.get_value() is False + + def is_set(self) -> bool: + return ( + get_config_bool_value( + *FEATURE_FLAGS_SECTION_PATH, key=self.value.name.lower(), default=None + ) + is not None + ) def env_variable(self): return get_env_variable_name(*FEATURE_FLAGS_SECTION_PATH, key=self.value.name) diff --git a/tests/api/test_feature_flags.py b/tests/api/test_feature_flags.py index 0b41a3626f..d4d9da8e9c 100644 --- a/tests/api/test_feature_flags.py +++ b/tests/api/test_feature_flags.py @@ -15,7 +15,6 @@ from unittest import mock import pytest -from click import ClickException from snowflake.cli.api.feature_flags import BooleanFlag, FeatureFlagMixin @@ -23,22 +22,36 @@ class _TestFlags(FeatureFlagMixin): # Intentional inconsistency between constant and the enum name to make sure there's no strict relation ENABLED_BY_DEFAULT = BooleanFlag("ENABLED_DEFAULT", True) DISABLED_BY_DEFAULT = BooleanFlag("DISABLED_DEFAULT", False) - NON_BOOLEAN = BooleanFlag("NON_BOOLEAN", "xys") # type: ignore + NON_BOOLEAN_DEFAULT = BooleanFlag("NON_BOOLEAN", "xys") # type: ignore + NONE_AS_DEFAULT = BooleanFlag("NON_BOOLEAN", "xys") # type: ignore -def test_flag_value_has_to_be_boolean(): - with pytest.raises(ClickException): - _TestFlags.NON_BOOLEAN.is_enabled() +def test_flag_value_default_non_boolean(): + _TestFlags.NON_BOOLEAN_DEFAULT.is_enabled() is False + _TestFlags.NON_BOOLEAN_DEFAULT.is_disabled() is False + _TestFlags.NON_BOOLEAN_DEFAULT.get_value() == "xys" + _TestFlags.NON_BOOLEAN_DEFAULT.is_set() is True + + +def test_flag_value_default_is_none(): + _TestFlags.NONE_AS_DEFAULT.is_enabled() is False + _TestFlags.NONE_AS_DEFAULT.is_disabled() is False + _TestFlags.NONE_AS_DEFAULT.get_value() is None + _TestFlags.NONE_AS_DEFAULT.is_set() is False def test_flag_is_enabled(): assert _TestFlags.ENABLED_BY_DEFAULT.is_enabled() is True assert _TestFlags.ENABLED_BY_DEFAULT.is_disabled() is False + assert _TestFlags.ENABLED_BY_DEFAULT.get_value() is True + assert _TestFlags.ENABLED_BY_DEFAULT.is_set() is False def test_flag_is_disabled(): assert _TestFlags.DISABLED_BY_DEFAULT.is_enabled() is False assert _TestFlags.DISABLED_BY_DEFAULT.is_disabled() is True + assert _TestFlags.DISABLED_BY_DEFAULT.get_value() is False + assert _TestFlags.DISABLED_BY_DEFAULT.is_set() is False def test_flag_env_variable_value(): @@ -53,13 +66,50 @@ def test_flag_env_variable_value(): @mock.patch("snowflake.cli.api.config.get_config_value") -@pytest.mark.parametrize("value_from_config", [True, False]) -def test_flag_from_config_file(mock_get_config_value, value_from_config): +@pytest.mark.parametrize("value_from_config", [True, False, None]) +def test_is_enabled_flag_from_config_file(mock_get_config_value, value_from_config): + mock_get_config_value.return_value = value_from_config + + assert _TestFlags.DISABLED_BY_DEFAULT.is_enabled() is (value_from_config or False) + mock_get_config_value.assert_called_once_with( + "cli", "features", key="disabled_default", default=None + ) + + +@mock.patch("snowflake.cli.api.config.get_config_value") +@pytest.mark.parametrize("value_from_config", [True, False, None]) +def test_is_disabled_flag_from_config_file(mock_get_config_value, value_from_config): + mock_get_config_value.return_value = value_from_config + + assert _TestFlags.DISABLED_BY_DEFAULT.is_disabled() is not ( + value_from_config or False + ) + mock_get_config_value.assert_called_once_with( + "cli", "features", key="disabled_default", default=None + ) + + +@mock.patch("snowflake.cli.api.config.get_config_value") +@pytest.mark.parametrize("value_from_config", [True, False, None]) +def test_is_set_flag_from_config_file(mock_get_config_value, value_from_config): mock_get_config_value.return_value = value_from_config - assert _TestFlags.DISABLED_BY_DEFAULT.is_enabled() is value_from_config + assert _TestFlags.DISABLED_BY_DEFAULT.is_set() is (value_from_config is not None) + + mock_get_config_value.assert_called_once_with( + "cli", "features", key="disabled_default", default=None + ) + + +@mock.patch("snowflake.cli.api.config.get_config_value") +@pytest.mark.parametrize("value_from_config", [True, False, None]) +def test_get_value_flag_from_config_file(mock_get_config_value, value_from_config): + mock_get_config_value.return_value = value_from_config + + assert _TestFlags.DISABLED_BY_DEFAULT.get_value() == (value_from_config or False) + mock_get_config_value.assert_called_once_with( - "cli", "features", key="disabled_default", default=False + "cli", "features", key="disabled_default", default=None ) diff --git a/tests/nativeapp/test_application_package_entity.py b/tests/nativeapp/test_application_package_entity.py index 3966a391ff..d07683ac94 100644 --- a/tests/nativeapp/test_application_package_entity.py +++ b/tests/nativeapp/test_application_package_entity.py @@ -17,6 +17,7 @@ from unittest import mock import yaml +from snowflake.cli._plugins.connection.util import UIParameter from snowflake.cli._plugins.nativeapp.constants import ( LOOSE_FILES_MAGIC_VERSION, SPECIAL_COMMENT, @@ -32,6 +33,7 @@ APP_PACKAGE_ENTITY, APPLICATION_PACKAGE_ENTITY_MODULE, SQL_EXECUTOR_EXECUTE, + SQL_FACADE_GET_UI_PARAMETER, mock_execute_helper, ) @@ -75,7 +77,9 @@ def test_bundle(project_directory): @mock.patch(f"{APP_PACKAGE_ENTITY}.execute_post_deploy_hooks") @mock.patch(f"{APP_PACKAGE_ENTITY}.validate_setup_script") @mock.patch(f"{APPLICATION_PACKAGE_ENTITY_MODULE}.sync_deploy_root_with_stage") +@mock.patch(SQL_FACADE_GET_UI_PARAMETER, return_value="ENABLED") def test_deploy( + mock_get_parameter, mock_sync, mock_validate, mock_execute_post_deploy_hooks, @@ -164,6 +168,9 @@ def test_deploy( ) mock_validate.assert_called_once() mock_execute_post_deploy_hooks.assert_called_once_with() + mock_get_parameter.assert_called_once_with( + UIParameter.NA_FEATURE_RELEASE_CHANNELS, "ENABLED" + ) assert mock_execute.mock_calls == expected diff --git a/tests/nativeapp/test_event_sharing.py b/tests/nativeapp/test_event_sharing.py index 463e58719e..8da67f6c29 100644 --- a/tests/nativeapp/test_event_sharing.py +++ b/tests/nativeapp/test_event_sharing.py @@ -419,8 +419,8 @@ def _setup_mocks_for_upgrade_app( @mock.patch( GET_UI_PARAMETERS, return_value={ - UIParameter.NA_EVENT_SHARING_V2: "false", - UIParameter.NA_ENFORCE_MANDATORY_FILTERS: "false", + UIParameter.NA_EVENT_SHARING_V2: False, + UIParameter.NA_ENFORCE_MANDATORY_FILTERS: False, }, ) @pytest.mark.parametrize( @@ -504,8 +504,8 @@ def test_event_sharing_disabled_no_change_to_current_behavior( @mock.patch( GET_UI_PARAMETERS, return_value={ - UIParameter.NA_EVENT_SHARING_V2: "false", - UIParameter.NA_ENFORCE_MANDATORY_FILTERS: "false", + UIParameter.NA_EVENT_SHARING_V2: False, + UIParameter.NA_ENFORCE_MANDATORY_FILTERS: False, }, ) @pytest.mark.parametrize( @@ -586,8 +586,8 @@ def test_event_sharing_disabled_but_we_add_event_sharing_flag_in_project_definit @mock.patch( GET_UI_PARAMETERS, return_value={ - UIParameter.NA_EVENT_SHARING_V2: "true", - UIParameter.NA_ENFORCE_MANDATORY_FILTERS: "false", + UIParameter.NA_EVENT_SHARING_V2: True, + UIParameter.NA_ENFORCE_MANDATORY_FILTERS: False, }, ) @pytest.mark.parametrize( @@ -663,8 +663,8 @@ def test_event_sharing_enabled_not_enforced_no_mandatory_events_then_flag_respec @mock.patch( GET_UI_PARAMETERS, return_value={ - UIParameter.NA_EVENT_SHARING_V2: "true", - UIParameter.NA_ENFORCE_MANDATORY_FILTERS: "true", + UIParameter.NA_EVENT_SHARING_V2: True, + UIParameter.NA_ENFORCE_MANDATORY_FILTERS: True, }, ) @pytest.mark.parametrize( @@ -740,8 +740,8 @@ def test_event_sharing_enabled_when_upgrade_flag_matches_existing_app_then_do_no @mock.patch( GET_UI_PARAMETERS, return_value={ - UIParameter.NA_EVENT_SHARING_V2: "true", - UIParameter.NA_ENFORCE_MANDATORY_FILTERS: "false", + UIParameter.NA_EVENT_SHARING_V2: True, + UIParameter.NA_ENFORCE_MANDATORY_FILTERS: False, }, ) @pytest.mark.parametrize( @@ -823,8 +823,8 @@ def test_event_sharing_enabled_with_mandatory_events_and_explicit_authorization_ @mock.patch( GET_UI_PARAMETERS, return_value={ - UIParameter.NA_EVENT_SHARING_V2: "true", - UIParameter.NA_ENFORCE_MANDATORY_FILTERS: "false", + UIParameter.NA_EVENT_SHARING_V2: True, + UIParameter.NA_ENFORCE_MANDATORY_FILTERS: False, }, ) @pytest.mark.parametrize( @@ -911,8 +911,8 @@ def test_event_sharing_enabled_with_mandatory_events_but_no_authorization_then_f @mock.patch( GET_UI_PARAMETERS, return_value={ - UIParameter.NA_EVENT_SHARING_V2: "true", - UIParameter.NA_ENFORCE_MANDATORY_FILTERS: "true", + UIParameter.NA_EVENT_SHARING_V2: True, + UIParameter.NA_ENFORCE_MANDATORY_FILTERS: True, }, ) @pytest.mark.parametrize( @@ -986,8 +986,8 @@ def test_enforced_events_sharing_with_no_mandatory_events_then_use_value_provide @mock.patch( GET_UI_PARAMETERS, return_value={ - UIParameter.NA_EVENT_SHARING_V2: "true", - UIParameter.NA_ENFORCE_MANDATORY_FILTERS: "true", + UIParameter.NA_EVENT_SHARING_V2: True, + UIParameter.NA_ENFORCE_MANDATORY_FILTERS: True, }, ) @pytest.mark.parametrize( @@ -1061,8 +1061,8 @@ def test_enforced_events_sharing_with_mandatory_events_and_authorization_provide @mock.patch( GET_UI_PARAMETERS, return_value={ - UIParameter.NA_EVENT_SHARING_V2: "true", - UIParameter.NA_ENFORCE_MANDATORY_FILTERS: "true", + UIParameter.NA_EVENT_SHARING_V2: True, + UIParameter.NA_ENFORCE_MANDATORY_FILTERS: True, }, ) @pytest.mark.parametrize( @@ -1147,8 +1147,8 @@ def test_enforced_events_sharing_with_mandatory_events_and_authorization_refused @mock.patch( GET_UI_PARAMETERS, return_value={ - UIParameter.NA_EVENT_SHARING_V2: "true", - UIParameter.NA_ENFORCE_MANDATORY_FILTERS: "true", + UIParameter.NA_EVENT_SHARING_V2: True, + UIParameter.NA_ENFORCE_MANDATORY_FILTERS: True, }, ) @pytest.mark.parametrize( @@ -1233,8 +1233,8 @@ def test_enforced_events_sharing_with_mandatory_events_manifest_and_authorizatio @mock.patch( GET_UI_PARAMETERS, return_value={ - UIParameter.NA_EVENT_SHARING_V2: "true", - UIParameter.NA_ENFORCE_MANDATORY_FILTERS: "true", + UIParameter.NA_EVENT_SHARING_V2: True, + UIParameter.NA_ENFORCE_MANDATORY_FILTERS: True, }, ) @pytest.mark.parametrize( @@ -1310,8 +1310,8 @@ def test_enforced_events_sharing_with_mandatory_events_and_dev_mode_then_default @mock.patch( GET_UI_PARAMETERS, return_value={ - UIParameter.NA_EVENT_SHARING_V2: "true", - UIParameter.NA_ENFORCE_MANDATORY_FILTERS: "true", + UIParameter.NA_EVENT_SHARING_V2: True, + UIParameter.NA_ENFORCE_MANDATORY_FILTERS: True, }, ) @pytest.mark.parametrize( @@ -1395,8 +1395,8 @@ def test_enforced_events_sharing_with_mandatory_events_and_authorization_not_spe @mock.patch( GET_UI_PARAMETERS, return_value={ - UIParameter.NA_EVENT_SHARING_V2: "true", - UIParameter.NA_ENFORCE_MANDATORY_FILTERS: "true", + UIParameter.NA_EVENT_SHARING_V2: True, + UIParameter.NA_ENFORCE_MANDATORY_FILTERS: True, }, ) @pytest.mark.parametrize( @@ -1476,8 +1476,8 @@ def test_enforced_events_sharing_with_mandatory_events_and_authorization_not_spe @mock.patch( GET_UI_PARAMETERS, return_value={ - UIParameter.NA_EVENT_SHARING_V2: "true", - UIParameter.NA_ENFORCE_MANDATORY_FILTERS: "true", + UIParameter.NA_EVENT_SHARING_V2: True, + UIParameter.NA_ENFORCE_MANDATORY_FILTERS: True, }, ) @pytest.mark.parametrize( @@ -1548,8 +1548,8 @@ def test_shared_events_with_no_enabled_mandatory_events_then_error( @mock.patch( GET_UI_PARAMETERS, return_value={ - UIParameter.NA_EVENT_SHARING_V2: "true", - UIParameter.NA_ENFORCE_MANDATORY_FILTERS: "true", + UIParameter.NA_EVENT_SHARING_V2: True, + UIParameter.NA_ENFORCE_MANDATORY_FILTERS: True, }, ) @pytest.mark.parametrize( diff --git a/tests/nativeapp/test_feature_flags.py b/tests/nativeapp/test_feature_flags.py index 921f87c7ba..6c40d47ad4 100644 --- a/tests/nativeapp/test_feature_flags.py +++ b/tests/nativeapp/test_feature_flags.py @@ -27,5 +27,5 @@ def test_feature_setup_script_generation_enabled( assert FeatureFlag.ENABLE_NATIVE_APP_PYTHON_SETUP.is_enabled() is value_from_config mock_get_config_value.assert_called_once_with( - "cli", "features", key="enable_native_app_python_setup", default=False + "cli", "features", key="enable_native_app_python_setup", default=None ) diff --git a/tests/nativeapp/test_manager.py b/tests/nativeapp/test_manager.py index 437bede8fb..57cafe7a07 100644 --- a/tests/nativeapp/test_manager.py +++ b/tests/nativeapp/test_manager.py @@ -80,9 +80,12 @@ APP_PACKAGE_ENTITY_IS_DISTRIBUTION_SAME, ENTITIES_UTILS_MODULE, SQL_EXECUTOR_EXECUTE, + SQL_FACADE_ALTER_APP_PKG_PROPERTIES, + SQL_FACADE_CREATE_APP_PKG, SQL_FACADE_CREATE_SCHEMA, SQL_FACADE_CREATE_STAGE, SQL_FACADE_GET_ACCOUNT_EVENT_TABLE, + SQL_FACADE_GET_UI_PARAMETER, SQL_FACADE_STAGE_EXISTS, mock_execute_helper, mock_snowflake_yml_file_v2, @@ -773,38 +776,21 @@ def test_get_snowsight_url_without_pdf_warehouse( # Test create_app_package() with no existing package available -@mock.patch(SQL_EXECUTOR_EXECUTE) @mock.patch(APP_PACKAGE_ENTITY_GET_EXISTING_APP_PKG_INFO, return_value=None) -def test_create_app_pkg_no_existing_package( +@mock.patch(SQL_FACADE_GET_UI_PARAMETER, return_value="ENABLED") +@mock.patch(SQL_FACADE_CREATE_APP_PKG) +@mock.patch("snowflake.cli.api.config.get_config_value") +@pytest.mark.parametrize("feature_flag", [True, False, None]) +def test_given_no_existing_pkg_when_create_app_pkg_then_success_and_respect_release_channels_flag( + mock_get_config_value, + mock_create_app_pkg, + mock_get_ui_parameter, mock_get_existing_app_pkg_info, - mock_execute, temp_dir, - mock_cursor, workspace_context, + feature_flag, ): - side_effects, expected = mock_execute_helper( - [ - ( - mock_cursor([("old_role",)], []), - mock.call("select current_role()"), - ), - (None, mock.call("use role package_role")), - ( - None, - mock.call( - dedent( - f"""\ - create application package app_pkg - comment = {SPECIAL_COMMENT} - distribution = internal - """ - ) - ), - ), - (None, mock.call("use role old_role")), - ] - ) - mock_execute.side_effect = side_effects + mock_get_config_value.return_value = feature_flag current_working_directory = os.getcwd() create_named_file( @@ -816,9 +802,70 @@ def test_create_app_pkg_no_existing_package( dm = _get_dm() pkg_model: ApplicationPackageEntityModel = dm.project_definition.entities["app_pkg"] pkg = ApplicationPackageEntity(pkg_model, workspace_context) + pkg.create_app_package() - assert mock_execute.mock_calls == expected + mock_get_existing_app_pkg_info.assert_called_once() + mock_create_app_pkg.assert_called_once_with( + package_name="app_pkg", + distribution="internal", + enable_release_channels=feature_flag, + role="package_role", + ) + mock_get_config_value.assert_called_once_with( + "cli", "features", key="enable_release_channels", default=None + ) + + +@mock.patch(APP_PACKAGE_ENTITY_GET_EXISTING_APP_PKG_INFO) +@mock_get_app_pkg_distribution_in_sf() +@mock.patch(APP_PACKAGE_ENTITY_IS_DISTRIBUTION_SAME) +@mock.patch(SQL_FACADE_GET_UI_PARAMETER, return_value="ENABLED") +@mock.patch(SQL_FACADE_ALTER_APP_PKG_PROPERTIES) +@mock.patch("snowflake.cli.api.config.get_config_value") +@pytest.mark.parametrize("feature_flag", [True, False, None]) +def test_given_existing_app_package_with_feature_flag_set_when_create_pkg_then_set_pkg_property_to_same_value( + mock_get_config_value, + mock_alter_app_pkg_properties, + mock_get_ui_parameter, + mock_is_distribution_same, + mock_get_distribution, + mock_get_existing_app_pkg_info, + temp_dir, + workspace_context, + feature_flag, +): + mock_get_config_value.return_value = feature_flag + mock_is_distribution_same.return_value = True + mock_get_distribution.return_value = "internal" + mock_get_existing_app_pkg_info.return_value = { + "name": "APP_PKG", + "comment": SPECIAL_COMMENT, + "version": LOOSE_FILES_MAGIC_VERSION, + "owner": "PACKAGE_ROLE", + } + + current_working_directory = os.getcwd() + create_named_file( + file_name="snowflake.yml", + dir_name=current_working_directory, + contents=[mock_snowflake_yml_file_v2], + ) + dm = _get_dm() + pkg_model: ApplicationPackageEntityModel = dm.project_definition.entities["app_pkg"] + pkg = ApplicationPackageEntity(pkg_model, workspace_context) + workspace_context.console = mock.MagicMock() + + pkg.create_app_package() + + mock_alter_app_pkg_properties.assert_called_once_with( + package_name="app_pkg", + enable_release_channels=feature_flag, + role="package_role", + ) + mock_get_config_value.assert_called_once_with( + "cli", "features", key="enable_release_channels", default=None + ) # Test create_app_package() with a different owner @@ -826,7 +873,9 @@ def test_create_app_pkg_no_existing_package( @mock.patch(APP_PACKAGE_ENTITY_GET_EXISTING_APP_PKG_INFO) @mock_get_app_pkg_distribution_in_sf() @mock.patch(APP_PACKAGE_ENTITY_IS_DISTRIBUTION_SAME, return_value=True) +@mock.patch(SQL_FACADE_GET_UI_PARAMETER, return_value="ENABLED") def test_create_app_pkg_different_owner( + mock_get_ui_parameter, mock_is_distribution_same, mock_get_distribution, mock_get_existing_app_pkg_info, @@ -869,7 +918,9 @@ def test_create_app_pkg_different_owner( "is_pkg_distribution_same", [False, True], ) +@mock.patch(SQL_FACADE_GET_UI_PARAMETER, return_value="ENABLED") def test_create_app_pkg_external_distribution( + mock_get_ui_parameter, mock_is_distribution_same, mock_get_distribution, mock_get_existing_app_pkg_info, @@ -917,7 +968,9 @@ def test_create_app_pkg_external_distribution( (True, SPECIAL_COMMENT_OLD), ], ) +@mock.patch(SQL_FACADE_GET_UI_PARAMETER, return_value="ENABLED") def test_create_app_pkg_internal_distribution_special_comment( + mock_get_ui_parameter, mock_is_distribution_same, mock_get_distribution, mock_get_existing_app_pkg_info, diff --git a/tests/nativeapp/test_run_processor.py b/tests/nativeapp/test_run_processor.py index dc6df30b4b..8d2bb4c545 100644 --- a/tests/nativeapp/test_run_processor.py +++ b/tests/nativeapp/test_run_processor.py @@ -232,8 +232,8 @@ def setup_project_file(current_working_directory: str, pdf=None): @mock.patch( GET_UI_PARAMETERS, return_value={ - UIParameter.NA_EVENT_SHARING_V2: "false", - UIParameter.NA_ENFORCE_MANDATORY_FILTERS: "false", + UIParameter.NA_EVENT_SHARING_V2: False, + UIParameter.NA_ENFORCE_MANDATORY_FILTERS: False, }, ) def test_create_dev_app_w_warehouse_access_exception( @@ -308,8 +308,8 @@ def test_create_dev_app_w_warehouse_access_exception( @mock.patch( GET_UI_PARAMETERS, return_value={ - UIParameter.NA_EVENT_SHARING_V2: "false", - UIParameter.NA_ENFORCE_MANDATORY_FILTERS: "false", + UIParameter.NA_EVENT_SHARING_V2: False, + UIParameter.NA_ENFORCE_MANDATORY_FILTERS: False, }, ) def test_create_dev_app_create_new_w_no_additional_privileges( @@ -361,8 +361,8 @@ def test_create_dev_app_create_new_w_no_additional_privileges( @mock.patch( GET_UI_PARAMETERS, return_value={ - UIParameter.NA_EVENT_SHARING_V2: "false", - UIParameter.NA_ENFORCE_MANDATORY_FILTERS: "false", + UIParameter.NA_EVENT_SHARING_V2: False, + UIParameter.NA_ENFORCE_MANDATORY_FILTERS: False, }, ) @pytest.mark.parametrize( @@ -450,8 +450,8 @@ def test_create_or_upgrade_dev_app_with_warning( @mock.patch( GET_UI_PARAMETERS, return_value={ - UIParameter.NA_EVENT_SHARING_V2: "false", - UIParameter.NA_ENFORCE_MANDATORY_FILTERS: "false", + UIParameter.NA_EVENT_SHARING_V2: False, + UIParameter.NA_ENFORCE_MANDATORY_FILTERS: False, }, ) def test_create_dev_app_create_new_with_additional_privileges( @@ -523,8 +523,8 @@ def test_create_dev_app_create_new_with_additional_privileges( @mock.patch( GET_UI_PARAMETERS, return_value={ - UIParameter.NA_EVENT_SHARING_V2: "false", - UIParameter.NA_ENFORCE_MANDATORY_FILTERS: "false", + UIParameter.NA_EVENT_SHARING_V2: False, + UIParameter.NA_ENFORCE_MANDATORY_FILTERS: False, }, ) def test_create_dev_app_create_new_w_missing_warehouse_exception( @@ -575,8 +575,8 @@ def test_create_dev_app_create_new_w_missing_warehouse_exception( @mock.patch( GET_UI_PARAMETERS, return_value={ - UIParameter.NA_EVENT_SHARING_V2: "false", - UIParameter.NA_ENFORCE_MANDATORY_FILTERS: "false", + UIParameter.NA_EVENT_SHARING_V2: False, + UIParameter.NA_ENFORCE_MANDATORY_FILTERS: False, }, ) @pytest.mark.parametrize( @@ -627,8 +627,8 @@ def test_create_dev_app_incorrect_properties( @mock.patch( GET_UI_PARAMETERS, return_value={ - UIParameter.NA_EVENT_SHARING_V2: "false", - UIParameter.NA_ENFORCE_MANDATORY_FILTERS: "false", + UIParameter.NA_EVENT_SHARING_V2: False, + UIParameter.NA_ENFORCE_MANDATORY_FILTERS: False, }, ) def test_create_dev_app_incorrect_owner( @@ -685,8 +685,8 @@ def test_create_dev_app_incorrect_owner( @mock.patch( GET_UI_PARAMETERS, return_value={ - UIParameter.NA_EVENT_SHARING_V2: "false", - UIParameter.NA_ENFORCE_MANDATORY_FILTERS: "false", + UIParameter.NA_EVENT_SHARING_V2: False, + UIParameter.NA_ENFORCE_MANDATORY_FILTERS: False, }, ) @mock_connection() @@ -742,8 +742,8 @@ def test_create_dev_app_no_diff_changes( @mock.patch( GET_UI_PARAMETERS, return_value={ - UIParameter.NA_EVENT_SHARING_V2: "false", - UIParameter.NA_ENFORCE_MANDATORY_FILTERS: "false", + UIParameter.NA_EVENT_SHARING_V2: False, + UIParameter.NA_ENFORCE_MANDATORY_FILTERS: False, }, ) def test_create_dev_app_w_diff_changes( @@ -797,8 +797,8 @@ def test_create_dev_app_w_diff_changes( @mock.patch( GET_UI_PARAMETERS, return_value={ - UIParameter.NA_EVENT_SHARING_V2: "false", - UIParameter.NA_ENFORCE_MANDATORY_FILTERS: "false", + UIParameter.NA_EVENT_SHARING_V2: False, + UIParameter.NA_ENFORCE_MANDATORY_FILTERS: False, }, ) def test_create_dev_app_recreate_w_missing_warehouse_exception( @@ -843,8 +843,8 @@ def test_create_dev_app_recreate_w_missing_warehouse_exception( @mock.patch( GET_UI_PARAMETERS, return_value={ - UIParameter.NA_EVENT_SHARING_V2: "false", - UIParameter.NA_ENFORCE_MANDATORY_FILTERS: "false", + UIParameter.NA_EVENT_SHARING_V2: False, + UIParameter.NA_ENFORCE_MANDATORY_FILTERS: False, }, ) def test_create_dev_app_create_new_quoted( @@ -922,8 +922,8 @@ def test_create_dev_app_create_new_quoted( @mock.patch( GET_UI_PARAMETERS, return_value={ - UIParameter.NA_EVENT_SHARING_V2: "false", - UIParameter.NA_ENFORCE_MANDATORY_FILTERS: "false", + UIParameter.NA_EVENT_SHARING_V2: False, + UIParameter.NA_ENFORCE_MANDATORY_FILTERS: False, }, ) def test_create_dev_app_create_new_quoted_override( @@ -985,8 +985,8 @@ def test_create_dev_app_create_new_quoted_override( @mock.patch( GET_UI_PARAMETERS, return_value={ - UIParameter.NA_EVENT_SHARING_V2: "false", - UIParameter.NA_ENFORCE_MANDATORY_FILTERS: "false", + UIParameter.NA_EVENT_SHARING_V2: False, + UIParameter.NA_ENFORCE_MANDATORY_FILTERS: False, }, ) def test_create_dev_app_recreate_app_when_orphaned( @@ -1105,8 +1105,8 @@ def test_create_dev_app_recreate_app_when_orphaned( @mock.patch( GET_UI_PARAMETERS, return_value={ - UIParameter.NA_EVENT_SHARING_V2: "false", - UIParameter.NA_ENFORCE_MANDATORY_FILTERS: "false", + UIParameter.NA_EVENT_SHARING_V2: False, + UIParameter.NA_ENFORCE_MANDATORY_FILTERS: False, }, ) def test_create_dev_app_recreate_app_when_orphaned_requires_cascade( @@ -1247,8 +1247,8 @@ def test_create_dev_app_recreate_app_when_orphaned_requires_cascade( @mock.patch( GET_UI_PARAMETERS, return_value={ - UIParameter.NA_EVENT_SHARING_V2: "false", - UIParameter.NA_ENFORCE_MANDATORY_FILTERS: "false", + UIParameter.NA_EVENT_SHARING_V2: False, + UIParameter.NA_ENFORCE_MANDATORY_FILTERS: False, }, ) def test_create_dev_app_recreate_app_when_orphaned_requires_cascade_unknown_objects( @@ -1374,8 +1374,8 @@ def test_create_dev_app_recreate_app_when_orphaned_requires_cascade_unknown_obje @mock.patch( GET_UI_PARAMETERS, return_value={ - UIParameter.NA_EVENT_SHARING_V2: "false", - UIParameter.NA_ENFORCE_MANDATORY_FILTERS: "false", + UIParameter.NA_EVENT_SHARING_V2: False, + UIParameter.NA_ENFORCE_MANDATORY_FILTERS: False, }, ) @pytest.mark.parametrize( @@ -1430,8 +1430,8 @@ def test_upgrade_app_warehouse_error( @mock.patch( GET_UI_PARAMETERS, return_value={ - UIParameter.NA_EVENT_SHARING_V2: "false", - UIParameter.NA_ENFORCE_MANDATORY_FILTERS: "false", + UIParameter.NA_EVENT_SHARING_V2: False, + UIParameter.NA_ENFORCE_MANDATORY_FILTERS: False, }, ) @pytest.mark.parametrize( @@ -1491,8 +1491,8 @@ def test_upgrade_app_incorrect_owner( @mock.patch( GET_UI_PARAMETERS, return_value={ - UIParameter.NA_EVENT_SHARING_V2: "false", - UIParameter.NA_ENFORCE_MANDATORY_FILTERS: "false", + UIParameter.NA_EVENT_SHARING_V2: False, + UIParameter.NA_ENFORCE_MANDATORY_FILTERS: False, }, ) @pytest.mark.parametrize( @@ -1547,8 +1547,8 @@ def test_upgrade_app_succeeds( @mock.patch( GET_UI_PARAMETERS, return_value={ - UIParameter.NA_EVENT_SHARING_V2: "false", - UIParameter.NA_ENFORCE_MANDATORY_FILTERS: "false", + UIParameter.NA_EVENT_SHARING_V2: False, + UIParameter.NA_ENFORCE_MANDATORY_FILTERS: False, }, ) @pytest.mark.parametrize( @@ -1610,8 +1610,8 @@ def test_upgrade_app_fails_generic_error( @mock.patch( GET_UI_PARAMETERS, return_value={ - UIParameter.NA_EVENT_SHARING_V2: "false", - UIParameter.NA_ENFORCE_MANDATORY_FILTERS: "false", + UIParameter.NA_EVENT_SHARING_V2: False, + UIParameter.NA_ENFORCE_MANDATORY_FILTERS: False, }, ) @pytest.mark.parametrize( @@ -1689,8 +1689,8 @@ def test_upgrade_app_fails_upgrade_restriction_error( @mock.patch( GET_UI_PARAMETERS, return_value={ - UIParameter.NA_EVENT_SHARING_V2: "false", - UIParameter.NA_ENFORCE_MANDATORY_FILTERS: "false", + UIParameter.NA_EVENT_SHARING_V2: False, + UIParameter.NA_ENFORCE_MANDATORY_FILTERS: False, }, ) def test_versioned_app_upgrade_to_unversioned( @@ -1807,8 +1807,8 @@ def test_versioned_app_upgrade_to_unversioned( @mock.patch( GET_UI_PARAMETERS, return_value={ - UIParameter.NA_EVENT_SHARING_V2: "false", - UIParameter.NA_ENFORCE_MANDATORY_FILTERS: "false", + UIParameter.NA_EVENT_SHARING_V2: False, + UIParameter.NA_ENFORCE_MANDATORY_FILTERS: False, }, ) @pytest.mark.parametrize( @@ -1891,8 +1891,8 @@ def test_upgrade_app_fails_drop_fails( @mock.patch( GET_UI_PARAMETERS, return_value={ - UIParameter.NA_EVENT_SHARING_V2: "false", - UIParameter.NA_ENFORCE_MANDATORY_FILTERS: "false", + UIParameter.NA_EVENT_SHARING_V2: False, + UIParameter.NA_ENFORCE_MANDATORY_FILTERS: False, }, ) @pytest.mark.parametrize("policy_param", [allow_always_policy, ask_always_policy]) @@ -2062,8 +2062,8 @@ def test_upgrade_app_from_version_throws_usage_error_two( @mock.patch( GET_UI_PARAMETERS, return_value={ - UIParameter.NA_EVENT_SHARING_V2: "false", - UIParameter.NA_ENFORCE_MANDATORY_FILTERS: "false", + UIParameter.NA_EVENT_SHARING_V2: False, + UIParameter.NA_ENFORCE_MANDATORY_FILTERS: False, }, ) @pytest.mark.parametrize("policy_param", [allow_always_policy, ask_always_policy]) diff --git a/tests/nativeapp/test_sf_sql_facade.py b/tests/nativeapp/test_sf_sql_facade.py index f1d3de9bb2..0e847108e5 100644 --- a/tests/nativeapp/test_sf_sql_facade.py +++ b/tests/nativeapp/test_sf_sql_facade.py @@ -17,6 +17,7 @@ from unittest.mock import _Call as Call import pytest +from snowflake.cli._plugins.connection.util import UIParameter from snowflake.cli._plugins.nativeapp.constants import ( AUTHORIZE_TELEMETRY_COL, COMMENT_COL, @@ -1274,16 +1275,6 @@ def test_get_app_properties_bubbles_errors(mock_execute_query): assert f"Failed to describe application {app_name}. {error_message}" in str(err) -expected_ui_params_query = dedent( - f""" - select value['value']::string as PARAM_VALUE, value['name']::string as PARAM_NAME from table(flatten( - input => parse_json(SYSTEM$BOOTSTRAP_DATA_REQUEST()), - path => 'clientParamsInfo' - )) where value['name'] in ('ENABLE_EVENT_SHARING_V2_IN_THE_SAME_ACCOUNT', 'ENFORCE_MANDATORY_FILTERS_FOR_SAME_ACCOUNT_INSTALLATION', 'UI_SNOWSIGHT_ENABLE_REGIONLESS_REDIRECT'); - """ -) - - @mock.patch(SQL_EXECUTOR_EXECUTE) @pytest.mark.parametrize( "events, expected_result", @@ -2332,3 +2323,327 @@ def test_create_application_converts_unexpected_programmingerrors_to_unclassifie ) assert_programmingerror_cause_with_errno(err, SQL_COMPILATION_ERROR) + + +@pytest.mark.parametrize( + "pkg_name, sanitized_pkg_name", + [("test_pkg", "test_pkg"), ("test.pkg", '"test.pkg"')], +) +def test_given_basic_pkg_when_create_application_package_then_success( + mock_execute_query, mock_use_role, pkg_name, sanitized_pkg_name +): + distribution = "INTERNAL" + role = "test_role" + + expected_use_objects = [(mock_use_role, mock.call(role))] + + expected_execute_query = [ + ( + mock_execute_query, + mock.call( + dedent( + f"""\ + create application package {sanitized_pkg_name} + comment = {SPECIAL_COMMENT} + distribution = {distribution} + """ + ).strip() + ), + ) + ] + with assert_in_context(expected_use_objects, expected_execute_query): + sql_facade.create_application_package(pkg_name, distribution, role=role) + + +@pytest.mark.parametrize("enable_release_channels", [True, False]) +def test_given_release_channels_when_create_application_package_then_success( + mock_execute_query, mock_use_role, enable_release_channels +): + package_name = "test_package" + distribution = "INTERNAL" + role = "test_role" + + expected_use_objects = [(mock_use_role, mock.call(role))] + + expected_execute_query = [ + ( + mock_execute_query, + mock.call( + dedent( + f"""\ + create application package {package_name} + comment = {SPECIAL_COMMENT} + distribution = {distribution} + enable_release_channels = {str(enable_release_channels).lower()} + """ + ).strip() + ), + ) + ] + with assert_in_context(expected_use_objects, expected_execute_query): + sql_facade.create_application_package( + package_name, + distribution, + role=role, + enable_release_channels=enable_release_channels, + ) + + +def test_given_programming_error_when_create_application_package_then_error( + mock_execute_query, + mock_use_role, +): + package_name = "test_package" + distribution = "INTERNAL" + role = "test_role" + + side_effects, expected = mock_execute_helper( + [ + ( + ProgrammingError(), + mock.call( + dedent( + f"""\ + create application package {package_name} + comment = {SPECIAL_COMMENT} + distribution = {distribution} + """ + ).strip() + ), + ) + ] + ) + mock_execute_query.side_effect = side_effects + + with pytest.raises(InvalidSQLError) as err: + sql_facade.create_application_package(package_name, distribution, role=role) + + assert "Failed to create application package" in str(err) + + +def test_given_privilege_error_when_create_application_package_then_raise_priv_error( + mock_execute_query, + mock_use_role, +): + package_name = "test_package" + distribution = "INTERNAL" + role = "test_role" + + side_effects, expected = mock_execute_helper( + [ + ( + ProgrammingError(errno=INSUFFICIENT_PRIVILEGES), + mock.call( + dedent( + f"""\ + create application package {package_name} + comment = {SPECIAL_COMMENT} + distribution = {distribution} + """ + ).strip() + ), + ) + ] + ) + mock_execute_query.side_effect = side_effects + + with pytest.raises(InsufficientPrivilegesError) as err: + sql_facade.create_application_package(package_name, distribution, role=role) + + assert "Insufficient privileges to create application package" in str(err) + + +@pytest.mark.parametrize( + "pkg_name, sanitized_pkg_name", + [("test_pkg", "test_pkg"), ("test.pkg", '"test.pkg"')], +) +@pytest.mark.parametrize("enable_release_channels", [True, False]) +def test_given_basic_pkg_when_update_application_package_properties_then_success( + mock_execute_query, + mock_use_role, + pkg_name, + sanitized_pkg_name, + enable_release_channels, +): + expected_use_objects = [(mock_use_role, mock.call(None))] + expected_execute_query = [ + ( + mock_execute_query, + mock.call( + dedent( + f"""\ + alter application package {sanitized_pkg_name} + set enable_release_channels = {str(enable_release_channels).lower()} + """ + ) + ), + ) + ] + with assert_in_context(expected_use_objects, expected_execute_query): + sql_facade.alter_application_package_properties( + pkg_name, enable_release_channels=enable_release_channels + ) + + +def test_given_no_enable_release_channel_flag_when_update_application_package_then_no_action( + mock_execute_query, +): + sql_facade.alter_application_package_properties("test_pkg", role="test_role") + + assert mock_execute_query.call_count == 0 + + +def test_given_programming_error_when_update_application_package_then_raise_sql_error( + mock_execute_query, mock_use_role +): + pkg_name = "test_pkg" + role = "test_role" + side_effects, expected = mock_execute_helper( + [ + ( + ProgrammingError(), + mock.call( + dedent( + f"""\ + alter application package {pkg_name} + set enable_release_channels = True + """ + ) + ), + ) + ] + ) + mock_execute_query.side_effect = side_effects + + with pytest.raises(InvalidSQLError) as err: + sql_facade.alter_application_package_properties( + pkg_name, enable_release_channels=True, role=role + ) + + assert "Failed to update enable_release_channels for application package" in str( + err + ) + + +def test_given_privilege_exception_when_update_application_package_then_raise_priv_error( + mock_execute_query, + mock_use_role, +): + pkg_name = "test_pkg" + role = "test_role" + side_effects, expected = mock_execute_helper( + [ + ( + ProgrammingError(errno=INSUFFICIENT_PRIVILEGES), + mock.call( + dedent( + f"""\ + alter application package {pkg_name} + set enable_release_channels = False + """ + ) + ), + ) + ] + ) + mock_execute_query.side_effect = side_effects + + with pytest.raises(InsufficientPrivilegesError) as err: + sql_facade.alter_application_package_properties( + pkg_name, enable_release_channels=False, role=role + ) + + assert ( + "Insufficient privileges update enable_release_channels for application package" + in str(err) + ) + + +expected_ui_params_query = "call system$bootstrap_data_request('CLIENT_PARAMS_INFO')" + + +def test_get_ui_parameter_with_value(mock_cursor): + with mock.patch.object(sql_facade, "_sql_executor") as mock_sql_executor: + execute_str_mock = mock_sql_executor._conn.execute_string # noqa: SLF001 + execute_str_mock.return_value = ( + None, + mock_cursor( + [ + ( + """\ + { + "clientParamsInfo": [{ + "name": "FEATURE_RELEASE_CHANNELS", + "value": true + }] + } + """, + ) + ], + [], + ), + ) + + assert ( + sql_facade.get_ui_parameter(UIParameter.NA_FEATURE_RELEASE_CHANNELS, False) + is True + ) + + execute_str_mock.assert_called_once_with(expected_ui_params_query) + + +def test_get_ui_parameter_with_empty_value_then_use_empty_value(mock_cursor): + with mock.patch.object(sql_facade, "_sql_executor") as mock_sql_executor: + execute_str_mock = mock_sql_executor._conn.execute_string # noqa: SLF001 + execute_str_mock.return_value = ( + None, + mock_cursor( + [ + ( + """\ + { + "clientParamsInfo": [{ + "name": "FEATURE_RELEASE_CHANNELS", + "value": "" + }] + } + """, + ) + ], + [], + ), + ) + + assert ( + sql_facade.get_ui_parameter(UIParameter.NA_FEATURE_RELEASE_CHANNELS, False) + == "" + ) + + execute_str_mock.assert_called_once_with(expected_ui_params_query) + + +def test_get_ui_parameter_with_no_value_then_use_default(mock_cursor): + with mock.patch.object(sql_facade, "_sql_executor") as mock_sql_executor: + execute_str_mock = mock_sql_executor._conn.execute_string # noqa: SLF001 + execute_str_mock.return_value = ( + None, + mock_cursor( + [ + ( + """\ + { + "clientParamsInfo": [] + } + """, + ) + ], + [], + ), + ) + + assert ( + sql_facade.get_ui_parameter(UIParameter.NA_FEATURE_RELEASE_CHANNELS, "any") + == "any" + ) + + execute_str_mock.assert_called_once_with(expected_ui_params_query) diff --git a/tests/nativeapp/utils.py b/tests/nativeapp/utils.py index 576fce4436..4a614bbbda 100644 --- a/tests/nativeapp/utils.py +++ b/tests/nativeapp/utils.py @@ -80,6 +80,11 @@ SQL_FACADE_GET_EVENT_DEFINITIONS = f"{SQL_FACADE}.get_event_definitions" SQL_FACADE_GET_EXISTING_APP_INFO = f"{SQL_FACADE}.get_existing_app_info" SQL_FACADE_GRANT_PRIVILEGES_TO_ROLE = f"{SQL_FACADE}.grant_privileges_to_role" +SQL_FACADE_GET_UI_PARAMETER = f"{SQL_FACADE}.get_ui_parameter" +SQL_FACADE_ALTER_APP_PKG_PROPERTIES = ( + f"{SQL_FACADE}.alter_application_package_properties" +) +SQL_FACADE_CREATE_APP_PKG = f"{SQL_FACADE}.create_application_package" mock_snowflake_yml_file = dedent( """\ diff --git a/tests/streamlit/test_commands.py b/tests/streamlit/test_commands.py index 68464053bd..f0d53541bd 100644 --- a/tests/streamlit/test_commands.py +++ b/tests/streamlit/test_commands.py @@ -66,7 +66,7 @@ def _put_query(source: str, dest: str): @mock.patch("snowflake.connector.connect") @mock.patch( GET_UI_PARAMETERS, - return_value={UIParameter.NA_ENABLE_REGIONLESS_REDIRECT: "false"}, + return_value={UIParameter.NA_ENABLE_REGIONLESS_REDIRECT: False}, ) @mock_streamlit_exists def test_deploy_only_streamlit_file( @@ -121,7 +121,7 @@ def test_deploy_only_streamlit_file( @mock.patch("snowflake.connector.connect") @mock.patch( GET_UI_PARAMETERS, - return_value={UIParameter.NA_ENABLE_REGIONLESS_REDIRECT: "false"}, + return_value={UIParameter.NA_ENABLE_REGIONLESS_REDIRECT: False}, ) @mock_streamlit_exists def test_deploy_only_streamlit_file_no_stage( @@ -175,7 +175,7 @@ def test_deploy_only_streamlit_file_no_stage( @mock.patch("snowflake.connector.connect") @mock.patch( GET_UI_PARAMETERS, - return_value={UIParameter.NA_ENABLE_REGIONLESS_REDIRECT: "false"}, + return_value={UIParameter.NA_ENABLE_REGIONLESS_REDIRECT: False}, ) @mock_streamlit_exists def test_deploy_with_empty_pages( @@ -231,7 +231,7 @@ def test_deploy_with_empty_pages( @mock.patch("snowflake.connector.connect") @mock.patch( GET_UI_PARAMETERS, - return_value={UIParameter.NA_ENABLE_REGIONLESS_REDIRECT: "false"}, + return_value={UIParameter.NA_ENABLE_REGIONLESS_REDIRECT: False}, ) @mock_streamlit_exists def test_deploy_only_streamlit_file_replace( @@ -302,7 +302,7 @@ def test_artifacts_must_exists( @mock.patch("snowflake.connector.connect") @mock.patch( GET_UI_PARAMETERS, - return_value={UIParameter.NA_ENABLE_REGIONLESS_REDIRECT: "false"}, + return_value={UIParameter.NA_ENABLE_REGIONLESS_REDIRECT: False}, ) @mock_streamlit_exists def test_deploy_launch_browser( @@ -337,7 +337,7 @@ def test_deploy_launch_browser( @mock.patch("snowflake.connector.connect") @mock.patch( GET_UI_PARAMETERS, - return_value={UIParameter.NA_ENABLE_REGIONLESS_REDIRECT: "false"}, + return_value={UIParameter.NA_ENABLE_REGIONLESS_REDIRECT: False}, ) @mock_streamlit_exists def test_deploy_streamlit_and_environment_files( @@ -382,7 +382,7 @@ def test_deploy_streamlit_and_environment_files( @mock.patch("snowflake.connector.connect") @mock.patch( GET_UI_PARAMETERS, - return_value={UIParameter.NA_ENABLE_REGIONLESS_REDIRECT: "false"}, + return_value={UIParameter.NA_ENABLE_REGIONLESS_REDIRECT: False}, ) @mock_streamlit_exists def test_deploy_streamlit_and_pages_files( @@ -426,7 +426,7 @@ def test_deploy_streamlit_and_pages_files( @mock.patch("snowflake.connector.connect") @mock.patch( GET_UI_PARAMETERS, - return_value={UIParameter.NA_ENABLE_REGIONLESS_REDIRECT: "false"}, + return_value={UIParameter.NA_ENABLE_REGIONLESS_REDIRECT: False}, ) @mock_streamlit_exists def test_deploy_all_streamlit_files( @@ -471,7 +471,7 @@ def test_deploy_all_streamlit_files( @mock.patch("snowflake.connector.connect") @mock.patch( GET_UI_PARAMETERS, - return_value={UIParameter.NA_ENABLE_REGIONLESS_REDIRECT: "false"}, + return_value={UIParameter.NA_ENABLE_REGIONLESS_REDIRECT: False}, ) @mock_streamlit_exists def test_deploy_put_files_on_stage( @@ -518,7 +518,7 @@ def test_deploy_put_files_on_stage( @mock.patch("snowflake.connector.connect") @mock.patch( GET_UI_PARAMETERS, - return_value={UIParameter.NA_ENABLE_REGIONLESS_REDIRECT: "false"}, + return_value={UIParameter.NA_ENABLE_REGIONLESS_REDIRECT: False}, ) @mock_streamlit_exists def test_deploy_all_streamlit_files_not_defaults( @@ -563,7 +563,7 @@ def test_deploy_all_streamlit_files_not_defaults( @pytest.mark.parametrize("enable_streamlit_no_checkouts", [True, False]) @mock.patch( GET_UI_PARAMETERS, - return_value={UIParameter.NA_ENABLE_REGIONLESS_REDIRECT: "false"}, + return_value={UIParameter.NA_ENABLE_REGIONLESS_REDIRECT: False}, ) @mock_streamlit_exists def test_deploy_streamlit_main_and_pages_files_experimental( @@ -640,7 +640,7 @@ def test_deploy_streamlit_main_and_pages_files_experimental( @mock.patch("snowflake.connector.connect") @mock.patch( GET_UI_PARAMETERS, - return_value={UIParameter.NA_ENABLE_REGIONLESS_REDIRECT: "false"}, + return_value={UIParameter.NA_ENABLE_REGIONLESS_REDIRECT: False}, ) @mock_streamlit_exists def test_deploy_streamlit_main_and_pages_files_experimental_double_deploy( @@ -707,7 +707,7 @@ def test_deploy_streamlit_main_and_pages_files_experimental_double_deploy( @pytest.mark.parametrize("enable_streamlit_versioned_stage", [True, False]) @mock.patch( GET_UI_PARAMETERS, - return_value={UIParameter.NA_ENABLE_REGIONLESS_REDIRECT: "false"}, + return_value={UIParameter.NA_ENABLE_REGIONLESS_REDIRECT: False}, ) @mock_streamlit_exists def test_deploy_streamlit_main_and_pages_files_experimental_no_stage( @@ -769,7 +769,7 @@ def test_deploy_streamlit_main_and_pages_files_experimental_no_stage( @mock.patch("snowflake.connector.connect") @mock.patch( GET_UI_PARAMETERS, - return_value={UIParameter.NA_ENABLE_REGIONLESS_REDIRECT: "false"}, + return_value={UIParameter.NA_ENABLE_REGIONLESS_REDIRECT: False}, ) @mock_streamlit_exists def test_deploy_streamlit_main_and_pages_files_experimental_replace( @@ -862,7 +862,7 @@ def test_drop_streamlit(mock_connector, runner, mock_ctx): @mock.patch("snowflake.connector.connect") @mock.patch( GET_UI_PARAMETERS, - return_value={UIParameter.NA_ENABLE_REGIONLESS_REDIRECT: "false"}, + return_value={UIParameter.NA_ENABLE_REGIONLESS_REDIRECT: False}, ) def test_get_streamlit_url(mock_param, mock_connector, mock_cursor, runner, mock_ctx): ctx = mock_ctx( @@ -944,7 +944,7 @@ def test_multiple_streamlit_raise_error_if_multiple_entities( @mock.patch("snowflake.connector.connect") @mock.patch( GET_UI_PARAMETERS, - return_value={UIParameter.NA_ENABLE_REGIONLESS_REDIRECT: "false"}, + return_value={UIParameter.NA_ENABLE_REGIONLESS_REDIRECT: False}, ) def test_deploy_streamlit_with_comment_v2( mock_param, mock_connector, mock_cursor, runner, mock_ctx, project_directory diff --git a/tests/test_utils.py b/tests/test_utils.py index 70e746dd14..10eadc479a 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -15,7 +15,6 @@ import json import os from pathlib import Path -from textwrap import dedent from unittest import mock from unittest.mock import MagicMock, patch @@ -37,7 +36,6 @@ from snowflake.cli.api.secure_path import SecurePath from snowflake.cli.api.utils import path_utils from snowflake.connector import SnowflakeConnection -from snowflake.connector.cursor import DictCursor from tests.test_data import test_data @@ -293,109 +291,153 @@ def test_get_host_region(host, expected): assert get_host_region(host) == expected -expected_ui_params_query = dedent( - f""" - select value['value']::string as PARAM_VALUE, value['name']::string as PARAM_NAME from table(flatten( - input => parse_json(SYSTEM$BOOTSTRAP_DATA_REQUEST()), - path => 'clientParamsInfo' - )) where value['name'] in ('ENABLE_EVENT_SHARING_V2_IN_THE_SAME_ACCOUNT', 'ENFORCE_MANDATORY_FILTERS_FOR_SAME_ACCOUNT_INSTALLATION', 'UI_SNOWSIGHT_ENABLE_REGIONLESS_REDIRECT'); - """ -) +expected_ui_params_query = "call system$bootstrap_data_request('CLIENT_PARAMS_INFO')" -def test_get_ui_parameters_no_param(): +def test_get_ui_parameters_no_param(mock_cursor): connection = MagicMock() - cursor = MagicMock() - connection.execute_string.return_value = (None, cursor) - cursor.fetchall.return_value = [] + connection.execute_string.return_value = ( + None, + mock_cursor([('{"clientParamsInfo": []}',)], []), + ) + assert get_ui_parameters(connection) == {} - connection.execute_string.assert_called_with( - expected_ui_params_query, cursor_class=DictCursor - ) + connection.execute_string.assert_called_with(expected_ui_params_query) -def test_get_ui_parameters_one_param(): +def test_get_ui_parameters_one_param(mock_cursor): connection = MagicMock() - cursor = MagicMock() - connection.execute_string.return_value = (None, cursor) - cursor.fetchall.return_value = [ - { - "PARAM_NAME": UIParameter.NA_ENABLE_REGIONLESS_REDIRECT.value, - "PARAM_VALUE": "true", - } - ] + connection.execute_string.return_value = ( + None, + mock_cursor( + [ + ( + """\ + { + "clientParamsInfo": [{ + "name": "UI_SNOWSIGHT_ENABLE_REGIONLESS_REDIRECT", + "value": true + }] + } + """, + ) + ], + [], + ), + ) + assert get_ui_parameters(connection) == { - UIParameter.NA_ENABLE_REGIONLESS_REDIRECT: "true" + UIParameter.NA_ENABLE_REGIONLESS_REDIRECT: True } - connection.execute_string.assert_called_with( - expected_ui_params_query, cursor_class=DictCursor - ) + connection.execute_string.assert_called_with(expected_ui_params_query) -def test_get_ui_parameters_multiple_params(): +def test_get_ui_parameters_multiple_params(mock_cursor): connection = MagicMock() - cursor = MagicMock() - connection.execute_string.return_value = (None, cursor) - cursor.fetchall.return_value = [ - { - "PARAM_NAME": UIParameter.NA_ENABLE_REGIONLESS_REDIRECT.value, - "PARAM_VALUE": "true", - }, - { - "PARAM_NAME": UIParameter.NA_EVENT_SHARING_V2.value, - "PARAM_VALUE": "false", - }, - ] + connection.execute_string.return_value = ( + None, + mock_cursor( + [ + ( + """\ + { + "clientParamsInfo": [{ + "name": "UI_SNOWSIGHT_ENABLE_REGIONLESS_REDIRECT", + "value": true + }, + { + "name": "ENABLE_EVENT_SHARING_V2_IN_THE_SAME_ACCOUNT", + "value": false + }] + } + """, + ) + ], + [], + ), + ) + assert get_ui_parameters(connection) == { - UIParameter.NA_ENABLE_REGIONLESS_REDIRECT: "true", - UIParameter.NA_EVENT_SHARING_V2: "false", + UIParameter.NA_ENABLE_REGIONLESS_REDIRECT: True, + UIParameter.NA_EVENT_SHARING_V2: False, } - connection.execute_string.assert_called_with( - expected_ui_params_query, cursor_class=DictCursor - ) + connection.execute_string.assert_called_with(expected_ui_params_query) -def test_get_ui_parameter_with_value(): +def test_get_ui_parameter_with_value(mock_cursor): connection = MagicMock() - cursor = MagicMock() - connection.execute_string.return_value = (None, cursor) - cursor.fetchall.return_value = [ - { - "PARAM_NAME": UIParameter.NA_ENABLE_REGIONLESS_REDIRECT.value, - "PARAM_VALUE": "true", - } - ] + connection.execute_string.return_value = ( + None, + mock_cursor( + [ + ( + """\ + { + "clientParamsInfo": [{ + "name": "UI_SNOWSIGHT_ENABLE_REGIONLESS_REDIRECT", + "value": true + }] + } + """, + ) + ], + [], + ), + ) assert ( - get_ui_parameter(connection, UIParameter.NA_ENABLE_REGIONLESS_REDIRECT, "false") - == "true" + get_ui_parameter(connection, UIParameter.NA_ENABLE_REGIONLESS_REDIRECT, False) + is True ) -def test_get_ui_parameter_with_empty_value_then_use_empty_value(): +def test_get_ui_parameter_with_empty_value_then_use_empty_value(mock_cursor): connection = MagicMock() - cursor = MagicMock() - connection.execute_string.return_value = (None, cursor) - cursor.fetchall.return_value = [ - { - "PARAM_NAME": UIParameter.NA_ENABLE_REGIONLESS_REDIRECT.value, - "PARAM_VALUE": "", - } - ] + connection.execute_string.return_value = ( + None, + mock_cursor( + [ + ( + """\ + { + "clientParamsInfo": [{ + "name": "UI_SNOWSIGHT_ENABLE_REGIONLESS_REDIRECT", + "value": "" + }] + } + """, + ) + ], + [], + ), + ) assert ( get_ui_parameter(connection, UIParameter.NA_ENABLE_REGIONLESS_REDIRECT, "false") == "" ) -def test_get_ui_parameter_with_no_value_then_use_default(): +def test_get_ui_parameter_with_no_value_then_use_default(mock_cursor): connection = MagicMock() - cursor = MagicMock() - connection.execute_string.return_value = (None, cursor) - cursor.fetchall.return_value = [] + connection.execute_string.return_value = ( + None, + mock_cursor( + [ + ( + """\ + { + "clientParamsInfo": [] + } + """, + ) + ], + [], + ), + ) + assert ( - get_ui_parameter(connection, UIParameter.NA_ENABLE_REGIONLESS_REDIRECT, "false") - == "false" + get_ui_parameter(connection, UIParameter.NA_ENABLE_REGIONLESS_REDIRECT, "any") + == "any" )