Skip to content

Commit

Permalink
Fix issue with list of accounts in release channels and directives (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
sfc-gh-melnacouzi authored Jan 17, 2025
1 parent 2da6caf commit b38a6dc
Show file tree
Hide file tree
Showing 6 changed files with 32 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1724,12 +1724,12 @@ def resolve_version_info(
if not self.get_existing_version_info(to_identifier(resolved_version)):
raise BadOptionUsage(
option_name="patch",
message=f"Cannot create patch {resolved_patch} when version {resolved_version} is not defined in the application package {self.name}. Try again without specifying a patch.",
message=f"Version {resolved_version} is not defined in the application package {self.name}. Try again with a patch of 0 or without specifying any patch.",
)
except ApplicationPackageDoesNotExistError as app_err:
raise BadOptionUsage(
option_name="patch",
message=f"Cannot create patch {resolved_patch} when application package {self.name} does not exist. Try again without specifying a patch.",
message=f"Application package {self.name} does not exist yet. Try again with a patch of 0 or without specifying any patch.",
)

return VersionInfo(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ def release_channel_add_accounts(
show_default=False,
help="The release channel to add accounts to.",
),
target_accounts: list[str] = typer.Option(
target_accounts: str = typer.Option(
show_default=False,
help="The accounts to add to the release channel. Format has to be `org1.account1,org2.account2`.",
),
Expand All @@ -100,7 +100,7 @@ def release_channel_add_accounts(
package_id,
EntityActions.RELEASE_CHANNEL_ADD_ACCOUNTS,
release_channel=channel,
target_accounts=target_accounts,
target_accounts=target_accounts.split(","),
)

return MessageResult("Successfully added accounts to the release channel.")
Expand All @@ -114,7 +114,7 @@ def release_channel_remove_accounts(
show_default=False,
help="The release channel to remove accounts from.",
),
target_accounts: list[str] = typer.Option(
target_accounts: str = typer.Option(
show_default=False,
help="The accounts to remove from the release channel. Format has to be `org1.account1,org2.account2`.",
),
Expand All @@ -134,7 +134,7 @@ def release_channel_remove_accounts(
package_id,
EntityActions.RELEASE_CHANNEL_REMOVE_ACCOUNTS,
release_channel=channel,
target_accounts=target_accounts,
target_accounts=target_accounts.split(","),
)

return MessageResult("Successfully removed accounts from the release channel.")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ def release_directive_set(
DEFAULT_CHANNEL,
help="Name of the release channel to use",
),
target_accounts: Optional[list[str]] = typer.Option(
target_accounts: Optional[str] = typer.Option(
None,
show_default=False,
help="List of the accounts to apply the release directive to. Format has to be `org1.account1,org2.account2`",
Expand Down Expand Up @@ -126,7 +126,7 @@ def release_directive_set(
release_directive=directive,
version=version,
patch=patch,
target_accounts=target_accounts,
target_accounts=None if target_accounts is None else target_accounts.split(","),
release_channel=channel,
)
return MessageResult("Successfully set release directive.")
Expand Down
21 changes: 13 additions & 8 deletions src/snowflake/cli/_plugins/nativeapp/sf_sql_facade.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@
RELEASE_DIRECTIVE_DOES_NOT_EXIST,
RELEASE_DIRECTIVES_VERSION_PATCH_NOT_FOUND,
SQL_COMPILATION_ERROR,
TARGET_ACCOUNT_USED_BY_OTHER_RELEASE_DIRECTIVE,
VERSION_ALREADY_ADDED_TO_RELEASE_CHANNEL,
VERSION_DOES_NOT_EXIST,
VERSION_NOT_ADDED_TO_RELEASE_CHANNEL,
Expand Down Expand Up @@ -737,7 +738,7 @@ def upgrade_application(
"""

name = to_identifier(name)
release_channel = to_identifier(release_channel) if release_channel else None
release_channel = to_identifier(release_channel or DEFAULT_CHANNEL)

install_method.ensure_app_usable(
app_name=name,
Expand All @@ -754,14 +755,14 @@ def get_app_properties():
with self._use_role_optional(role), self._use_warehouse_optional(warehouse):
try:
using_clause = install_method.using_clause(path_to_version_directory)
if release_channel:
current_release_channel = get_app_properties().get(
CHANNEL_COL, DEFAULT_CHANNEL

current_release_channel = (
get_app_properties().get(CHANNEL_COL) or DEFAULT_CHANNEL
)
if unquote_identifier(release_channel) != current_release_channel:
raise UpgradeApplicationRestrictionError(
f"Application {name} is currently on release channel {current_release_channel}. Cannot upgrade to release channel {release_channel}."
)
if not same_identifiers(release_channel, current_release_channel):
raise UpgradeApplicationRestrictionError(
f"Application {name} is currently on release channel {current_release_channel}. Cannot upgrade to release channel {release_channel}."
)

upgrade_cursor = self._sql_executor.execute_query(
f"alter application {name} upgrade {using_clause}",
Expand Down Expand Up @@ -1095,6 +1096,10 @@ def set_release_directive(
raise UserInputError(
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
if err.errno == TARGET_ACCOUNT_USED_BY_OTHER_RELEASE_DIRECTIVE:
raise UserInputError(
f"Some target accounts are already referenced by other release directives in application package {package_name}.\n{str(err.msg)}"
) from err
_handle_release_directive_version_error(
err,
package_name=package_name,
Expand Down
1 change: 1 addition & 0 deletions src/snowflake/cli/api/errno.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@
MAX_VERSIONS_IN_RELEASE_CHANNEL_REACHED = 512004
MAX_UNBOUND_VERSIONS_REACHED = 512023
CANNOT_DEREGISTER_VERSION_ASSOCIATED_WITH_CHANNEL = 512021
TARGET_ACCOUNT_USED_BY_OTHER_RELEASE_DIRECTIVE = 93091


ERR_JAVASCRIPT_EXECUTION = 100132
Expand Down
11 changes: 10 additions & 1 deletion tests/nativeapp/test_sf_sql_facade.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@
RELEASE_DIRECTIVE_DOES_NOT_EXIST,
RELEASE_DIRECTIVES_VERSION_PATCH_NOT_FOUND,
SQL_COMPILATION_ERROR,
TARGET_ACCOUNT_USED_BY_OTHER_RELEASE_DIRECTIVE,
VERSION_DOES_NOT_EXIST,
VERSION_NOT_ADDED_TO_RELEASE_CHANNEL,
VERSION_NOT_IN_RELEASE_CHANNEL,
Expand Down Expand Up @@ -1847,6 +1848,7 @@ def test_upgrade_application_unversioned(
mock_get_existing_app_info,
mock_use_warehouse,
mock_use_role,
mock_get_app_properties,
mock_execute_query,
mock_cursor,
):
Expand Down Expand Up @@ -1985,6 +1987,7 @@ def test_upgrade_application_converts_expected_programmingerrors_to_user_errors(
mock_get_existing_app_info,
mock_use_warehouse,
mock_use_role,
mock_get_app_properties,
mock_execute_query,
):
app_name = "test_app"
Expand Down Expand Up @@ -2101,6 +2104,7 @@ def test_upgrade_application_converts_unexpected_programmingerrors_to_unclassifi
mock_get_existing_app_info,
mock_use_warehouse,
mock_use_role,
mock_get_app_properties,
mock_execute_query,
):
app_name = "test_app"
Expand Down Expand Up @@ -2159,7 +2163,7 @@ def test_upgrade_application_with_release_channel_same_as_app_properties(
mock_get_app_properties.return_value = {
COMMENT_COL: SPECIAL_COMMENT,
AUTHORIZE_TELEMETRY_COL: "true",
CHANNEL_COL: release_channel,
CHANNEL_COL: release_channel.upper(),
}

side_effects, expected = mock_execute_helper(
Expand Down Expand Up @@ -3343,6 +3347,11 @@ def test_set_default_release_directive_no_release_channel(
UserInputError,
"Release directive test_directive does not exist in application package test_package.",
),
(
ProgrammingError(errno=TARGET_ACCOUNT_USED_BY_OTHER_RELEASE_DIRECTIVE),
UserInputError,
"Some target accounts are already referenced by other release directives in application package test_package.",
),
(
ProgrammingError(),
InvalidSQLError,
Expand Down

0 comments on commit b38a6dc

Please sign in to comment.