From 4ca052602c16c1f963f13f09fed8d85b9c8ebd68 Mon Sep 17 00:00:00 2001 From: Michel El Nacouzi Date: Fri, 10 Jan 2025 13:34:12 -0500 Subject: [PATCH] Bug fixes and improvements to release directives and channels commands (#1983) --- .../nativeapp/entities/application_package.py | 41 ++--- .../cli/_plugins/nativeapp/sf_sql_facade.py | 111 +++--------- .../test_application_package_entity.ambr | 8 +- .../test_application_package_entity.py | 90 +++++----- tests/nativeapp/test_run_processor.py | 2 +- tests/nativeapp/test_sf_sql_facade.py | 158 ++++-------------- tests/nativeapp/utils.py | 1 - 7 files changed, 121 insertions(+), 290 deletions(-) diff --git a/src/snowflake/cli/_plugins/nativeapp/entities/application_package.py b/src/snowflake/cli/_plugins/nativeapp/entities/application_package.py index 0a5bd308ac..5eab789add 100644 --- a/src/snowflake/cli/_plugins/nativeapp/entities/application_package.py +++ b/src/snowflake/cli/_plugins/nativeapp/entities/application_package.py @@ -672,7 +672,7 @@ def validate_release_channel( f"Release channels are not enabled for application package {self.name}." ) for channel in available_release_channels: - if same_identifiers(release_channel, channel["name"]): + if unquote_identifier(release_channel) == channel["name"]: return raise UsageError( @@ -736,30 +736,15 @@ def action_release_directive_set( sanitized_release_channel = self.get_sanitized_release_channel(release_channel) - if ( - not same_identifiers(release_directive, DEFAULT_DIRECTIVE) - and not target_accounts - ): - # if it is a non-default release directive with no target accounts specified, - # it means that the user wants to modify existing release directive - get_snowflake_facade().modify_release_directive( - package_name=self.name, - release_directive=release_directive, - release_channel=sanitized_release_channel, - version=version, - patch=patch, - role=self.role, - ) - else: - get_snowflake_facade().set_release_directive( - package_name=self.name, - release_directive=release_directive, - release_channel=sanitized_release_channel, - target_accounts=target_accounts, - version=version, - patch=patch, - role=self.role, - ) + get_snowflake_facade().set_release_directive( + package_name=self.name, + release_directive=release_directive, + release_channel=sanitized_release_channel, + target_accounts=target_accounts, + version=version, + patch=patch, + role=self.role, + ) def action_release_directive_unset( self, @@ -834,7 +819,7 @@ def action_release_channel_list( channel for channel in available_channels if release_channel is None - or same_identifiers(channel["name"], release_channel) + or unquote_identifier(release_channel) == channel["name"] ] if not filtered_channels: @@ -1069,12 +1054,12 @@ def action_publish( if not available_patches: raise ClickException( - f"Version {version} does not exist in application package {self.name}." + f"Version {version} does not exist in application package {self.name}. Use --create-version flag to create a new version." ) if patch not in available_patches: raise ClickException( - f"Patch {patch} does not exist for version {version} in application package {self.name}." + f"Patch {patch} does not exist for version {version} in application package {self.name}. Use --create-version flag to add a new patch." ) available_release_channels = get_snowflake_facade().show_release_channels( diff --git a/src/snowflake/cli/_plugins/nativeapp/sf_sql_facade.py b/src/snowflake/cli/_plugins/nativeapp/sf_sql_facade.py index ca4d113268..abffdb2649 100644 --- a/src/snowflake/cli/_plugins/nativeapp/sf_sql_facade.py +++ b/src/snowflake/cli/_plugins/nativeapp/sf_sql_facade.py @@ -333,7 +333,8 @@ def create_version_in_package( if err.errno == MAX_UNBOUND_VERSIONS_REACHED: raise UserInputError( f"Maximum unbound versions reached for application package {package_name}. " - "Please drop the other unbound version first, or add it to a release channel." + "Please drop other unbound versions first, or add them to a release channel. " + "Use `snow app version list` to view all versions.", ) from err if err.errno == APPLICATION_PACKAGE_MAX_VERSIONS_HIT: raise UserInputError( @@ -414,6 +415,8 @@ def add_patch_to_package_version( ) patch_query = f" {patch}" if patch is not None else "" + + # No space between patch and patch{patch_query} to avoid extra space when patch is None add_patch_query = dedent( f"""\ alter application package {package_name} @@ -670,6 +673,10 @@ def show_release_directives( f"Insufficient privileges to show release directives for application package {package_name}", role=role, ) from err + if err.errno == DOES_NOT_EXIST_OR_NOT_AUTHORIZED: + raise UserInputError( + f"Application package {package_name} does not exist or you are not authorized to access it." + ) from err handle_unclassified_error( err, f"Failed to show release directives for application package {package_name}.", @@ -1027,29 +1034,26 @@ def set_release_directive( @param [Optional] role: Role to switch to while running this script. Current role will be used if no role is passed in. """ - if same_identifiers(release_directive, DEFAULT_DIRECTIVE) and target_accounts: - raise UserInputError( - "Default release directive does not support target accounts." - ) - - if ( - not same_identifiers(release_directive, DEFAULT_DIRECTIVE) - and not target_accounts - ): - raise UserInputError( - "Non-default release directives require target accounts to be specified." - ) - package_name = to_identifier(package_name) release_channel = to_identifier(release_channel) if release_channel else None release_directive = to_identifier(release_directive) version = to_identifier(version) - release_directive_statement = ( - "set default release directive" - if same_identifiers(release_directive, DEFAULT_DIRECTIVE) - else f"set release directive {release_directive}" - ) + if same_identifiers(release_directive, DEFAULT_DIRECTIVE): + if target_accounts: + raise UserInputError( + "Default release directive does not support target accounts." + ) + release_directive_statement = "set default release directive" + else: + if target_accounts: + release_directive_statement = ( + f"set release directive {release_directive}" + ) + else: + release_directive_statement = ( + f"modify release directive {release_directive}" + ) release_channel_statement = ( f"modify release channel {release_channel}" if release_channel else "" @@ -1083,74 +1087,9 @@ def set_release_directive( raise UserInputError( f"Invalid account passed in.\n{str(err.msg)}" ) from err - _handle_release_directive_version_error( - err, - package_name=package_name, - release_channel=release_channel, - version=version, - patch=patch, - ) - handle_unclassified_error( - err, - f"Failed to set release directive {release_directive} for application package {package_name}.", - ) - - def modify_release_directive( - self, - package_name: str, - release_directive: str, - release_channel: str | None, - version: str, - patch: int, - role: str | None = None, - ): - """ - Modifies a release directive for an application package. - Release directive must already exist in the application package. - Accepts both default and non-default release directives. - - @param package_name: Name of the application package to alter. - @param release_directive: Name of the release directive to modify. - @param release_channel: Name of the release channel to modify the release directive for. - @param version: Version to modify the release directive for. - @param patch: Patch number to modify the release directive for. - @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) - release_channel = to_identifier(release_channel) if release_channel else None - release_directive = to_identifier(release_directive) - version = to_identifier(version) - - release_directive_statement = ( - "modify default release directive" - if same_identifiers(release_directive, DEFAULT_DIRECTIVE) - else f"modify release directive {release_directive}" - ) - - release_channel_statement = ( - f"modify release channel {release_channel}" if release_channel else "" - ) - - full_query = dedent( - _strip_empty_lines( - f"""\ - alter application package {package_name} - {release_channel_statement} - {release_directive_statement} - version = {version} patch = {patch} - """ - ) - ) - - with self._use_role_optional(role): - try: - self._sql_executor.execute_query(full_query) - except Exception as err: - if isinstance(err, ProgrammingError): if err.errno == RELEASE_DIRECTIVE_DOES_NOT_EXIST: raise UserInputError( - f"Release directive {release_directive} does not exist in application package {package_name}. Please create it first by specifying the target accounts." + f"Release directive {release_directive} does not exist in application package {package_name}. Please create it first by specifying --target-accounts with the `snow app release-directive set` command." ) from err _handle_release_directive_version_error( err, @@ -1161,7 +1100,7 @@ def modify_release_directive( ) handle_unclassified_error( err, - f"Failed to modify release directive {release_directive} for application package {package_name}.", + f"Failed to set release directive {release_directive} for application package {package_name}.", ) def unset_release_directive( diff --git a/tests/nativeapp/__snapshots__/test_application_package_entity.ambr b/tests/nativeapp/__snapshots__/test_application_package_entity.ambr index 30b56e7b2d..1ce48c6560 100644 --- a/tests/nativeapp/__snapshots__/test_application_package_entity.ambr +++ b/tests/nativeapp/__snapshots__/test_application_package_entity.ambr @@ -7,7 +7,7 @@ # --- # name: test_given_release_channel_with_no_target_account_or_version_then_show_all_accounts_in_snapshot ''' - channel1 + CHANNEL1 Description: desc Versions: () Created on: 2024-12-03 00:00:00.000000 UTC @@ -18,7 +18,7 @@ # --- # name: test_given_release_channels_with_a_selected_channel_to_filter_when_list_release_channels_then_returned_selected_channel ''' - channel1 + CHANNEL1 Description: desc Versions: (v1, v2) Created on: 2024-12-03 00:00:00.000000 UTC @@ -29,13 +29,13 @@ # --- # name: test_given_release_channels_with_proper_values_when_list_release_channels_then_success ''' - channel1 + CHANNEL1 Description: desc Versions: (v1, v2) Created on: 2024-12-03 00:00:00.000000 UTC Updated on: 2024-12-05 00:00:00.000000 UTC Target accounts: (org1.acc1, org2.acc2) - channel2 + CHANNEL2 Description: desc2 Versions: (v3) Created on: 2024-12-03 00:00:00.000000 UTC diff --git a/tests/nativeapp/test_application_package_entity.py b/tests/nativeapp/test_application_package_entity.py index 5d088d7f7b..09d1154000 100644 --- a/tests/nativeapp/test_application_package_entity.py +++ b/tests/nativeapp/test_application_package_entity.py @@ -42,7 +42,6 @@ SQL_FACADE_ADD_ACCOUNTS_TO_RELEASE_CHANNEL, SQL_FACADE_ADD_VERSION_TO_RELEASE_CHANNEL, SQL_FACADE_GET_UI_PARAMETER, - SQL_FACADE_MODIFY_RELEASE_DIRECTIVE, SQL_FACADE_REMOVE_ACCOUNTS_FROM_RELEASE_CHANNEL, SQL_FACADE_REMOVE_VERSION_FROM_RELEASE_CHANNEL, SQL_FACADE_SET_RELEASE_DIRECTIVE, @@ -234,7 +233,7 @@ def test_given_channels_disabled_and_no_directives_when_release_directive_list_t @mock.patch(SQL_FACADE_SHOW_RELEASE_CHANNELS, return_value=[]) -@mock.patch(SQL_FACADE_SHOW_RELEASE_DIRECTIVES, return_value=[{"name": "my_directive"}]) +@mock.patch(SQL_FACADE_SHOW_RELEASE_DIRECTIVES, return_value=[{"name": "MY_DIRECTIVE"}]) def test_given_channels_disabled_and_directives_present_when_release_directive_list_then_success( show_release_directives, show_release_channels, @@ -249,7 +248,7 @@ def test_given_channels_disabled_and_directives_present_when_release_directive_l action_ctx=action_context, release_channel=None, like="%%" ) - assert result == [{"name": "my_directive"}] + assert result == [{"name": "MY_DIRECTIVE"}] show_release_directives.assert_called_once_with( package_name=pkg_model.fqn.name, @@ -286,8 +285,8 @@ def test_given_multiple_directives_and_like_pattern_when_release_directive_list_ ) -@mock.patch(SQL_FACADE_SHOW_RELEASE_CHANNELS, return_value=[{"name": "my_channel"}]) -@mock.patch(SQL_FACADE_SHOW_RELEASE_DIRECTIVES, return_value=[{"name": "my_directive"}]) +@mock.patch(SQL_FACADE_SHOW_RELEASE_CHANNELS, return_value=[{"name": "MY_CHANNEL"}]) +@mock.patch(SQL_FACADE_SHOW_RELEASE_DIRECTIVES, return_value=[{"name": "MY_DIRECTIVE"}]) def test_given_channels_enabled_and_no_channel_specified_when_release_directive_list_then_success( show_release_directives, show_release_channels, @@ -302,7 +301,7 @@ def test_given_channels_enabled_and_no_channel_specified_when_release_directive_ action_ctx=action_context, release_channel=None, like="%%" ) - assert result == [{"name": "my_directive"}] + assert result == [{"name": "MY_DIRECTIVE"}] show_release_directives.assert_called_once_with( package_name=pkg_model.fqn.name, @@ -368,8 +367,8 @@ def test_given_channels_disabled_and_non_default_channel_selected_when_release_d show_release_directives.assert_not_called() -@mock.patch(SQL_FACADE_SHOW_RELEASE_CHANNELS, return_value=[{"name": "my_channel"}]) -@mock.patch(SQL_FACADE_SHOW_RELEASE_DIRECTIVES, return_value=[{"name": "my_directive"}]) +@mock.patch(SQL_FACADE_SHOW_RELEASE_CHANNELS, return_value=[{"name": "MY_CHANNEL"}]) +@mock.patch(SQL_FACADE_SHOW_RELEASE_DIRECTIVES, return_value=[{"name": "MY_DIRECTIVE"}]) def test_given_channels_enabled_and_invalid_channel_selected_when_release_directive_list_then_error( show_release_directives, show_release_channels, @@ -387,7 +386,7 @@ def test_given_channels_enabled_and_invalid_channel_selected_when_release_direct assert ( str(e.value) - == f"Release channel invalid_channel is not available in application package {pkg_model.fqn.name}. Available release channels are: (my_channel)." + == f"Release channel invalid_channel is not available in application package {pkg_model.fqn.name}. Available release channels are: (MY_CHANNEL)." ) show_release_channels.assert_called_once_with( pkg_model.fqn.name, pkg_model.meta.role @@ -396,8 +395,8 @@ def test_given_channels_enabled_and_invalid_channel_selected_when_release_direct show_release_directives.assert_not_called() -@mock.patch(SQL_FACADE_SHOW_RELEASE_CHANNELS, return_value=[{"name": "my_channel"}]) -@mock.patch(SQL_FACADE_SHOW_RELEASE_DIRECTIVES, return_value=[{"name": "my_directive"}]) +@mock.patch(SQL_FACADE_SHOW_RELEASE_CHANNELS, return_value=[{"name": "MY_CHANNEL"}]) +@mock.patch(SQL_FACADE_SHOW_RELEASE_DIRECTIVES, return_value=[{"name": "MY_DIRECTIVE"}]) def test_given_channels_enabled_and_valid_channel_selected_when_release_directive_list_then_success( show_release_directives, show_release_channels, @@ -412,7 +411,7 @@ def test_given_channels_enabled_and_valid_channel_selected_when_release_directiv action_ctx=action_context, release_channel="my_channel", like="%%" ) - assert result == [{"name": "my_directive"}] + assert result == [{"name": "MY_DIRECTIVE"}] show_release_channels.assert_called_once_with( pkg_model.fqn.name, pkg_model.meta.role @@ -425,7 +424,7 @@ def test_given_channels_enabled_and_valid_channel_selected_when_release_directiv ) -@mock.patch(SQL_FACADE_SHOW_RELEASE_CHANNELS, return_value=[{"name": "test_channel"}]) +@mock.patch(SQL_FACADE_SHOW_RELEASE_CHANNELS, return_value=[{"name": "TEST_CHANNEL"}]) @mock.patch(SQL_FACADE_SET_RELEASE_DIRECTIVE) def test_given_named_directive_with_accounts_when_release_directive_set_then_success( set_release_directive, @@ -460,7 +459,7 @@ def test_given_named_directive_with_accounts_when_release_directive_set_then_suc ) -@mock.patch(SQL_FACADE_SHOW_RELEASE_CHANNELS, return_value=[{"name": "test_channel"}]) +@mock.patch(SQL_FACADE_SHOW_RELEASE_CHANNELS, return_value=[{"name": "TEST_CHANNEL"}]) @mock.patch(SQL_FACADE_SET_RELEASE_DIRECTIVE) def test_given_default_directive_with_no_accounts_when_release_directive_set_then_success( set_release_directive, @@ -563,10 +562,10 @@ def test_given_no_channels_with_non_default_channel_used_when_release_directive_ set_release_directive.assert_not_called() -@mock.patch(SQL_FACADE_SHOW_RELEASE_CHANNELS, return_value=[{"name": "test_channel"}]) -@mock.patch(SQL_FACADE_MODIFY_RELEASE_DIRECTIVE) +@mock.patch(SQL_FACADE_SHOW_RELEASE_CHANNELS, return_value=[{"name": "TEST_CHANNEL"}]) +@mock.patch(SQL_FACADE_SET_RELEASE_DIRECTIVE) def test_given_named_directive_with_no_accounts_when_release_directive_set_then_modify_existing_directive( - modify_release_directive, + set_release_directive, show_release_channels, application_package_entity, action_context, @@ -587,17 +586,18 @@ def test_given_named_directive_with_no_accounts_when_release_directive_set_then_ pkg_model.fqn.name, pkg_model.meta.role ) - modify_release_directive.assert_called_once_with( + set_release_directive.assert_called_once_with( package_name=pkg_model.fqn.name, role=pkg_model.meta.role, version="1.0", patch=2, release_channel="test_channel", release_directive="directive", + target_accounts=None, ) -@mock.patch(SQL_FACADE_SHOW_RELEASE_CHANNELS, return_value=[{"name": "test_channel"}]) +@mock.patch(SQL_FACADE_SHOW_RELEASE_CHANNELS, return_value=[{"name": "TEST_CHANNEL"}]) @mock.patch(SQL_FACADE_SET_RELEASE_DIRECTIVE) def test_given_default_directive_with_accounts_when_release_directive_set_then_error( set_release_directive, @@ -627,7 +627,7 @@ def test_given_default_directive_with_accounts_when_release_directive_set_then_e # test with target_account not in org.account format: -@mock.patch(SQL_FACADE_SHOW_RELEASE_CHANNELS, return_value=[{"name": "test_channel"}]) +@mock.patch(SQL_FACADE_SHOW_RELEASE_CHANNELS, return_value=[{"name": "TEST_CHANNEL"}]) @mock.patch(SQL_FACADE_SET_RELEASE_DIRECTIVE) @pytest.mark.parametrize( "account_name", ["org1", "org1.", ".account1", "org1.acc.ount1"] @@ -663,7 +663,7 @@ def test_given_invalid_account_names_when_release_directive_set_then_error( @mock.patch( SQL_FACADE_SHOW_RELEASE_CHANNELS, - return_value=[{"name": "my_channel"}, {"name": "default"}], + return_value=[{"name": "MY_CHANNEL"}, {"name": "DEFAULT"}], ) @mock.patch(SQL_FACADE_UNSET_RELEASE_DIRECTIVE) def test_given_channels_enabled_and_default_channel_selected_when_release_directive_unset_then_success( @@ -693,7 +693,7 @@ def test_given_channels_enabled_and_default_channel_selected_when_release_direct ) -@mock.patch(SQL_FACADE_SHOW_RELEASE_CHANNELS, return_value=[{"name": "my_channel"}]) +@mock.patch(SQL_FACADE_SHOW_RELEASE_CHANNELS, return_value=[{"name": "MY_CHANNEL"}]) @mock.patch(SQL_FACADE_UNSET_RELEASE_DIRECTIVE) def test_given_channels_enabled_and_non_default_channel_selected_when_release_directive_unset_then_success( unset_release_directive, @@ -781,7 +781,7 @@ def test_given_channels_disabled_and_non_default_channel_selected_when_release_d unset_release_directive.assert_not_called() -@mock.patch(SQL_FACADE_SHOW_RELEASE_CHANNELS, return_value=[{"name": "my_channel"}]) +@mock.patch(SQL_FACADE_SHOW_RELEASE_CHANNELS, return_value=[{"name": "MY_CHANNEL"}]) @mock.patch(SQL_FACADE_UNSET_RELEASE_DIRECTIVE) def test_given_channels_enabled_and_non_existing_channel_selected_when_release_directive_unset_then_error( unset_release_directive, @@ -801,7 +801,7 @@ def test_given_channels_enabled_and_non_existing_channel_selected_when_release_d assert ( str(e.value) - == f"Release channel non_existing is not available in application package {pkg_model.fqn.name}. Available release channels are: (my_channel)." + == f"Release channel non_existing is not available in application package {pkg_model.fqn.name}. Available release channels are: (MY_CHANNEL)." ) show_release_channels.assert_called_once_with( @@ -863,7 +863,7 @@ def test_given_release_channels_with_proper_values_when_list_release_channels_th release_channels = [ { - "name": "channel1", + "name": "CHANNEL1", "description": "desc", "created_on": created_on_mock, "updated_on": updated_on_mock, @@ -871,7 +871,7 @@ def test_given_release_channels_with_proper_values_when_list_release_channels_th "targets": {"accounts": ["org1.acc1", "org2.acc2"]}, }, { - "name": "channel2", + "name": "CHANNEL2", "description": "desc2", "created_on": created_on_mock, "updated_on": updated_on_mock, @@ -914,7 +914,7 @@ def test_given_release_channel_with_no_target_account_or_version_then_show_all_a release_channels = [ { - "name": "channel1", + "name": "CHANNEL1", "description": "desc", "created_on": created_on_mock, "updated_on": updated_on_mock, @@ -981,7 +981,7 @@ def test_given_release_channels_with_a_selected_channel_to_filter_when_list_rele ) test_channel_1 = { - "name": "channel1", + "name": "CHANNEL1", "description": "desc", "created_on": created_on_mock, "updated_on": updated_on_mock, @@ -990,7 +990,7 @@ def test_given_release_channels_with_a_selected_channel_to_filter_when_list_rele } test_channel_2 = { - "name": "channel2", + "name": "CHANNEL2", "description": "desc2", "created_on": created_on_mock, "updated_on": updated_on_mock, @@ -1021,7 +1021,7 @@ def test_given_release_channel_and_accounts_when_add_accounts_to_release_channel pkg_model = application_package_entity._entity_model # noqa SLF001 pkg_model.meta.role = "package_role" - show_release_channels.return_value = [{"name": "test_channel"}] + show_release_channels.return_value = [{"name": "TEST_CHANNEL"}] application_package_entity.action_release_channel_add_accounts( action_ctx=action_context, @@ -1076,7 +1076,7 @@ def test_given_invalid_release_channel_when_add_accounts_to_release_channel_then pkg_model = application_package_entity._entity_model # noqa SLF001 pkg_model.meta.role = "package_role" - show_release_channels.return_value = [{"name": "test_channel"}] + show_release_channels.return_value = [{"name": "TEST_CHANNEL"}] with pytest.raises(UsageError) as e: application_package_entity.action_release_channel_add_accounts( @@ -1087,7 +1087,7 @@ def test_given_invalid_release_channel_when_add_accounts_to_release_channel_then assert ( str(e.value) - == f"Release channel invalid_channel is not available in application package {pkg_model.fqn.name}. Available release channels are: (test_channel)." + == f"Release channel invalid_channel is not available in application package {pkg_model.fqn.name}. Available release channels are: (TEST_CHANNEL)." ) add_accounts_to_release_channel.assert_not_called() @@ -1108,7 +1108,7 @@ def test_given_invalid_account_names_when_add_accounts_to_release_channel_then_e pkg_model = application_package_entity._entity_model # noqa SLF001 pkg_model.meta.role = "package_role" - show_release_channels.return_value = [{"name": "test_channel"}] + show_release_channels.return_value = [{"name": "TEST_CHANNEL"}] with pytest.raises(ClickException) as e: application_package_entity.action_release_channel_add_accounts( @@ -1136,7 +1136,7 @@ def test_given_release_channel_and_accounts_when_remove_accounts_from_release_ch pkg_model = application_package_entity._entity_model # noqa SLF001 pkg_model.meta.role = "package_role" - show_release_channels.return_value = [{"name": "test_channel"}] + show_release_channels.return_value = [{"name": "TEST_CHANNEL"}] application_package_entity.action_release_channel_remove_accounts( action_ctx=action_context, @@ -1191,7 +1191,7 @@ def test_given_invalid_release_channel_when_remove_accounts_from_release_channel pkg_model = application_package_entity._entity_model # noqa SLF001 pkg_model.meta.role = "package_role" - show_release_channels.return_value = [{"name": "test_channel"}] + show_release_channels.return_value = [{"name": "TEST_CHANNEL"}] with pytest.raises(UsageError) as e: application_package_entity.action_release_channel_remove_accounts( @@ -1202,7 +1202,7 @@ def test_given_invalid_release_channel_when_remove_accounts_from_release_channel assert ( str(e.value) - == f"Release channel invalid_channel is not available in application package {pkg_model.fqn.name}. Available release channels are: (test_channel)." + == f"Release channel invalid_channel is not available in application package {pkg_model.fqn.name}. Available release channels are: (TEST_CHANNEL)." ) remove_accounts_from_release_channel.assert_not_called() @@ -1223,7 +1223,7 @@ def test_given_invalid_account_names_when_remove_accounts_from_release_channel_t pkg_model = application_package_entity._entity_model # noqa SLF001 pkg_model.meta.role = "package_role" - show_release_channels.return_value = [{"name": "test_channel"}] + show_release_channels.return_value = [{"name": "TEST_CHANNEL"}] with pytest.raises(ClickException) as e: application_package_entity.action_release_channel_remove_accounts( @@ -1251,7 +1251,7 @@ def test_given_release_channel_and_version_when_release_channel_add_version_then pkg_model = application_package_entity._entity_model # noqa SLF001 pkg_model.meta.role = "package_role" - show_release_channels.return_value = [{"name": "test_channel"}] + show_release_channels.return_value = [{"name": "TEST_CHANNEL"}] application_package_entity.action_release_channel_add_version( action_ctx=action_context, @@ -1310,7 +1310,7 @@ def test_given_invalid_release_channel_when_release_channel_add_version_then_err pkg_model = application_package_entity._entity_model # noqa SLF001 pkg_model.meta.role = "package_role" - show_release_channels.return_value = [{"name": "test_channel"}] + show_release_channels.return_value = [{"name": "TEST_CHANNEL"}] with pytest.raises(UsageError) as e: application_package_entity.action_release_channel_add_version( @@ -1321,7 +1321,7 @@ def test_given_invalid_release_channel_when_release_channel_add_version_then_err assert ( str(e.value) - == f"Release channel invalid_channel is not available in application package {pkg_model.fqn.name}. Available release channels are: (test_channel)." + == f"Release channel invalid_channel is not available in application package {pkg_model.fqn.name}. Available release channels are: (TEST_CHANNEL)." ) add_version_to_release_channel.assert_not_called() @@ -1338,7 +1338,7 @@ def test_given_release_channel_and_version_when_release_channel_remove_version_t pkg_model = application_package_entity._entity_model # noqa SLF001 pkg_model.meta.role = "package_role" - show_release_channels.return_value = [{"name": "test_channel"}] + show_release_channels.return_value = [{"name": "TEST_CHANNEL"}] application_package_entity.action_release_channel_remove_version( action_ctx=action_context, @@ -1397,7 +1397,7 @@ def test_given_invalid_release_channel_when_release_channel_remove_version_then_ pkg_model = application_package_entity._entity_model # noqa SLF001 pkg_model.meta.role = "package_role" - show_release_channels.return_value = [{"name": "test_channel"}] + show_release_channels.return_value = [{"name": "TEST_CHANNEL"}] with pytest.raises(UsageError) as e: application_package_entity.action_release_channel_remove_version( @@ -1408,7 +1408,7 @@ def test_given_invalid_release_channel_when_release_channel_remove_version_then_ assert ( str(e.value) - == f"Release channel invalid_channel is not available in application package {pkg_model.fqn.name}. Available release channels are: (test_channel)." + == f"Release channel invalid_channel is not available in application package {pkg_model.fqn.name}. Available release channels are: (TEST_CHANNEL)." ) remove_version_from_release_channel.assert_not_called() @@ -1653,7 +1653,7 @@ def test_given_non_existing_version_when_publish_then_error( assert ( str(e.value) - == f"Version 1.0 does not exist in application package {pkg_model.fqn.name}." + == f"Version 1.0 does not exist in application package {pkg_model.fqn.name}. Use --create-version flag to create a new version." ) show_versions.assert_called_once_with(pkg_model.fqn.name, pkg_model.meta.role) @@ -1698,7 +1698,7 @@ def test_given_non_existing_patch_when_publish_then_error( assert ( str(e.value) - == f"Patch 1 does not exist for version 1.0 in application package {pkg_model.fqn.name}." + == f"Patch 1 does not exist for version 1.0 in application package {pkg_model.fqn.name}. Use --create-version flag to add a new patch." ) show_versions.assert_called_once_with(pkg_model.fqn.name, pkg_model.meta.role) diff --git a/tests/nativeapp/test_run_processor.py b/tests/nativeapp/test_run_processor.py index 90aa4b7cf8..34da87a798 100644 --- a/tests/nativeapp/test_run_processor.py +++ b/tests/nativeapp/test_run_processor.py @@ -2253,7 +2253,7 @@ def test_run_app_from_release_directive_with_channel( "comment": SPECIAL_COMMENT, "owner": "app_role", } - mock_show_release_channels.return_value = [{"name": "my_channel"}] + mock_show_release_channels.return_value = [{"name": "MY_CHANNEL"}] side_effects, expected = mock_execute_helper( [ ( diff --git a/tests/nativeapp/test_sf_sql_facade.py b/tests/nativeapp/test_sf_sql_facade.py index f473c43bdb..815df516dc 100644 --- a/tests/nativeapp/test_sf_sql_facade.py +++ b/tests/nativeapp/test_sf_sql_facade.py @@ -2913,6 +2913,11 @@ def test_show_release_directive_with_special_characters_in_names( InsufficientPrivilegesError, "Insufficient privileges to show release directives for application package test_package", ), + ( + ProgrammingError(errno=DOES_NOT_EXIST_OR_NOT_AUTHORIZED), + UserInputError, + "Application package test_package does not exist or you are not authorized to access it.", + ), ( DatabaseError("some database error"), UnknownSQLError, @@ -3333,6 +3338,11 @@ def test_set_default_release_directive_no_release_channel( UserInputError, "Invalid account passed in.", ), + ( + ProgrammingError(errno=RELEASE_DIRECTIVE_DOES_NOT_EXIST), + UserInputError, + "Release directive test_directive does not exist in application package test_package.", + ), ( ProgrammingError(), InvalidSQLError, @@ -3381,40 +3391,13 @@ def test_modify_release_directive_with_non_default_directive( """ ) - sql_facade.modify_release_directive( - package_name, - release_directive, - release_channel, - version, - patch, - ) - - mock_execute_query.assert_called_once_with(expected_query) - - -def test_modify_release_directive_with_default_directive( - mock_execute_query, -): - package_name = "test_package" - release_directive = "DEFAULT" - release_channel = "test_channel" - version = "1.0.0" - patch = 1 - expected_query = dedent( - f"""\ - alter application package {package_name} - modify release channel {release_channel} - modify default release directive - version = "{version}" patch = {patch} - """ - ) - - sql_facade.modify_release_directive( - package_name, - release_directive, - release_channel, - version, - patch, + sql_facade.set_release_directive( + package_name=package_name, + release_directive=release_directive, + release_channel=release_channel, + version=version, + patch=patch, + target_accounts=None, ) mock_execute_query.assert_called_once_with(expected_query) @@ -3437,12 +3420,13 @@ def test_modify_release_directive_with_special_chars_in_names( """ ) - sql_facade.modify_release_directive( - package_name, - release_directive, - release_channel, - version, - patch, + sql_facade.set_release_directive( + package_name=package_name, + release_directive=release_directive, + release_channel=release_channel, + version=version, + patch=patch, + target_accounts=None, ) mock_execute_query.assert_called_once_with(expected_query) @@ -3463,95 +3447,18 @@ def test_modify_release_directive_no_release_channel( """ ) - sql_facade.modify_release_directive( - package_name, - release_directive, - None, - version, - patch, - ) - - mock_execute_query.assert_called_once_with(expected_query) - - -def test_modify_default_release_directive_no_release_channel( - mock_execute_query, -): - package_name = "test_package" - release_directive = "DEFAULT" - version = "1.0.0" - patch = 1 - expected_query = dedent( - f"""\ - alter application package {package_name} - modify default release directive - version = "{version}" patch = {patch} - """ - ) - - sql_facade.modify_release_directive( - package_name, - release_directive, - None, - version, - patch, + sql_facade.set_release_directive( + package_name=package_name, + release_directive=release_directive, + release_channel=None, + version=version, + patch=patch, + target_accounts=None, ) mock_execute_query.assert_called_once_with(expected_query) -@pytest.mark.parametrize( - "error_raised, error_caught, error_message", - [ - ( - ProgrammingError(errno=VERSION_NOT_ADDED_TO_RELEASE_CHANNEL), - UserInputError, - 'Version "1.0.0" is not added to release channel test_channel. Please add it to the release channel first.', - ), - ( - ProgrammingError(errno=RELEASE_DIRECTIVES_VERSION_PATCH_NOT_FOUND), - UserInputError, - 'Patch 1 for version "1.0.0" not found in application package test_package.', - ), - ( - ProgrammingError(errno=VERSION_DOES_NOT_EXIST), - UserInputError, - 'Version "1.0.0" does not exist in application package test_package.', - ), - ( - ProgrammingError(errno=RELEASE_DIRECTIVE_DOES_NOT_EXIST), - UserInputError, - "Release directive test_directive does not exist in application package test_package.", - ), - ( - ProgrammingError(), - InvalidSQLError, - "Failed to modify release directive test_directive for application package test_package.", - ), - ( - DatabaseError("some database error"), - UnknownSQLError, - "Unknown SQL error occurred. Failed to modify release directive test_directive for application package test_package. some database error", - ), - ], -) -def test_modify_release_directive_errors( - mock_execute_query, error_raised, error_caught, error_message -): - mock_execute_query.side_effect = error_raised - - with pytest.raises(error_caught) as err: - sql_facade.modify_release_directive( - "test_package", - "test_directive", - "test_channel", - "1.0.0", - 1, - ) - - assert error_message in str(err) - - @contextmanager def mock_release_channels(facade, enabled): with mock.patch.object( @@ -3688,7 +3595,8 @@ def test_create_version_with_special_characters( ProgrammingError(errno=MAX_UNBOUND_VERSIONS_REACHED), UserInputError, "Maximum unbound versions reached for application package test_package. " - "Please drop the other unbound version first, or add it to a release channel.", + "Please drop other unbound versions first, or add them to a release channel. " + "Use `snow app version list` to view all versions.", ), ( ProgrammingError(errno=APPLICATION_PACKAGE_MAX_VERSIONS_HIT), diff --git a/tests/nativeapp/utils.py b/tests/nativeapp/utils.py index 353731ec0e..e0a0f948ff 100644 --- a/tests/nativeapp/utils.py +++ b/tests/nativeapp/utils.py @@ -91,7 +91,6 @@ SQL_FACADE_CREATE_APP_PKG = f"{SQL_FACADE}.create_application_package" SQL_FACADE_SHOW_RELEASE_DIRECTIVES = f"{SQL_FACADE}.show_release_directives" SQL_FACADE_SET_RELEASE_DIRECTIVE = f"{SQL_FACADE}.set_release_directive" -SQL_FACADE_MODIFY_RELEASE_DIRECTIVE = f"{SQL_FACADE}.modify_release_directive" SQL_FACADE_UNSET_RELEASE_DIRECTIVE = f"{SQL_FACADE}.unset_release_directive" SQL_FACADE_SHOW_RELEASE_CHANNELS = f"{SQL_FACADE}.show_release_channels" SQL_FACADE_DROP_VERSION = f"{SQL_FACADE}.drop_version_from_package"