From 78826c11121ee27c721bcd2673799384d0e6e098 Mon Sep 17 00:00:00 2001 From: Michel El Nacouzi Date: Mon, 9 Dec 2024 14:59:48 -0500 Subject: [PATCH] Address comments --- RELEASE-NOTES.md | 5 ++- .../cli/_plugins/nativeapp/constants.py | 3 ++ .../nativeapp/entities/application_package.py | 45 ++++++++++--------- .../nativeapp/release_directive/commands.py | 5 ++- .../cli/_plugins/nativeapp/sf_sql_facade.py | 14 +++--- tests/nativeapp/factories.py | 1 - 6 files changed, 44 insertions(+), 29 deletions(-) diff --git a/RELEASE-NOTES.md b/RELEASE-NOTES.md index 02a402bace..f02f630ba9 100644 --- a/RELEASE-NOTES.md +++ b/RELEASE-NOTES.md @@ -19,7 +19,10 @@ ## Deprecations ## New additions -* Add Release Directives support, through `snow app release-directive` commands. +* Add Release Directives support by introducing the following commands: + * `snow app release-directive list` + * `snow app release-directive set` + * `snow app release-directive unset` ## Fixes and improvements * Fixed crashes with older x86_64 Intel CPUs. diff --git a/src/snowflake/cli/_plugins/nativeapp/constants.py b/src/snowflake/cli/_plugins/nativeapp/constants.py index 0abe66beea..829e2751f9 100644 --- a/src/snowflake/cli/_plugins/nativeapp/constants.py +++ b/src/snowflake/cli/_plugins/nativeapp/constants.py @@ -27,3 +27,6 @@ INTERNAL_DISTRIBUTION = "internal" EXTERNAL_DISTRIBUTION = "external" + +DEFAULT_CHANNEL = "DEFAULT" +DEFAULT_DIRECTIVE = "DEFAULT" diff --git a/src/snowflake/cli/_plugins/nativeapp/entities/application_package.py b/src/snowflake/cli/_plugins/nativeapp/entities/application_package.py index 46ad4b0244..35d5963f73 100644 --- a/src/snowflake/cli/_plugins/nativeapp/entities/application_package.py +++ b/src/snowflake/cli/_plugins/nativeapp/entities/application_package.py @@ -21,6 +21,8 @@ from snowflake.cli._plugins.nativeapp.constants import ( ALLOWED_SPECIAL_COMMENTS, COMMENT_COL, + DEFAULT_CHANNEL, + DEFAULT_DIRECTIVE, EXTERNAL_DISTRIBUTION, INTERNAL_DISTRIBUTION, NAME_COL, @@ -562,19 +564,19 @@ def action_release_directive_list( If `like` is provided, only release directives matching the SQL LIKE pattern are listed. """ - release_channels = get_snowflake_facade().show_release_channels( + available_release_channels = get_snowflake_facade().show_release_channels( self.name, self.role ) # assume no release channel used if user selects default channel and release channels are not enabled if ( release_channel - and same_identifiers(release_channel, "DEFAULT") - and not release_channels + and same_identifiers(release_channel, DEFAULT_CHANNEL) + and not available_release_channels ): release_channel = None - release_channel_names = [c.get("name") for c in release_channels] + release_channel_names = [c.get("name") for c in available_release_channels] if release_channel and not identifier_in_list( release_channel, release_channel_names ): @@ -620,26 +622,29 @@ def action_release_directive_set( f"Target account {account} is not in a valid format. Make sure you provide the target account in the format 'org.account'." ) - if target_accounts and same_identifiers(release_directive, "DEFAULT"): + if target_accounts and same_identifiers(release_directive, DEFAULT_DIRECTIVE): raise BadOptionUsage( "target_accounts", "Target accounts can only be specified for non-default named release directives.", ) - release_channels = get_snowflake_facade().show_release_channels( + available_release_channels = get_snowflake_facade().show_release_channels( self.name, self.role ) - release_channel_names = [c.get("name") for c in release_channels] + release_channel_names = [c.get("name") for c in available_release_channels] - if not same_identifiers(release_channel, "DEFAULT") and not identifier_in_list( - release_channel, release_channel_names - ): + if not same_identifiers( + release_channel, DEFAULT_CHANNEL + ) and not identifier_in_list(release_channel, release_channel_names): raise ClickException( f"Release channel {release_channel} does not exist in application package {self.name}." ) - if not same_identifiers(release_directive, "DEFAULT") and not target_accounts: + 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( @@ -654,7 +659,7 @@ def action_release_directive_set( get_snowflake_facade().set_release_directive( package_name=self.name, release_directive=release_directive, - release_channel=release_channel if release_channels else None, + release_channel=release_channel if available_release_channels else None, target_accounts=target_accounts, version=version, patch=patch, @@ -667,18 +672,18 @@ def action_release_directive_unset( """ Unsets a release directive from the specified release channel. """ - if same_identifiers(release_directive, "DEFAULT"): + if same_identifiers(release_directive, DEFAULT_DIRECTIVE): raise ClickException( "Cannot unset default release directive. Please specify a non-default release directive." ) - release_channels = get_snowflake_facade().show_release_channels( + available_release_channels = get_snowflake_facade().show_release_channels( self.name, self.role ) - release_channel_names = [c.get("name") for c in release_channels] - if not same_identifiers(release_channel, "DEFAULT") and not identifier_in_list( - release_channel, release_channel_names - ): + release_channel_names = [c.get("name") for c in available_release_channels] + if not same_identifiers( + release_channel, DEFAULT_CHANNEL + ) and not identifier_in_list(release_channel, release_channel_names): raise ClickException( f"Release channel {release_channel} does not exist in application package {self.name}." ) @@ -686,7 +691,7 @@ def action_release_directive_unset( get_snowflake_facade().unset_release_directive( package_name=self.name, release_directive=release_directive, - release_channel=release_channel if release_channels else None, + release_channel=release_channel if available_release_channels else None, role=self.role, ) @@ -1206,7 +1211,7 @@ def resolve_version_info( # Check if patch needs to throw a bad option error, either if application package does not exist or if version does not exist if resolved_patch is not None: try: - if not self.get_existing_version_info(resolved_version): + 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.", diff --git a/src/snowflake/cli/_plugins/nativeapp/release_directive/commands.py b/src/snowflake/cli/_plugins/nativeapp/release_directive/commands.py index 3baeec6be2..2573bfdfe9 100644 --- a/src/snowflake/cli/_plugins/nativeapp/release_directive/commands.py +++ b/src/snowflake/cli/_plugins/nativeapp/release_directive/commands.py @@ -18,6 +18,7 @@ from typing import Optional import typer +from snowflake.cli._plugins.nativeapp.constants import DEFAULT_CHANNEL from snowflake.cli._plugins.nativeapp.v2_conversions.compat import ( force_project_definition_v2, ) @@ -88,7 +89,7 @@ def release_directive_set( help="Name of the release directive to set", ), channel: str = typer.Option( - "DEFAULT", + DEFAULT_CHANNEL, help="Name of the release channel to use", ), target_accounts: Optional[list[str]] = typer.Option( @@ -140,7 +141,7 @@ def release_directive_unset( help="Name of the release directive", ), channel: Optional[str] = typer.Option( - "DEFAULT", + DEFAULT_CHANNEL, help="Name of the release channel to use", ), **options, diff --git a/src/snowflake/cli/_plugins/nativeapp/sf_sql_facade.py b/src/snowflake/cli/_plugins/nativeapp/sf_sql_facade.py index ad089766c9..9bbed2b4cc 100644 --- a/src/snowflake/cli/_plugins/nativeapp/sf_sql_facade.py +++ b/src/snowflake/cli/_plugins/nativeapp/sf_sql_facade.py @@ -21,6 +21,7 @@ from snowflake.cli._plugins.connection.util import UIParameter, get_ui_parameter from snowflake.cli._plugins.nativeapp.constants import ( AUTHORIZE_TELEMETRY_COL, + DEFAULT_DIRECTIVE, NAME_COL, SPECIAL_COMMENT, ) @@ -881,12 +882,15 @@ 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") and target_accounts: + 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") and not 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." ) @@ -898,7 +902,7 @@ def set_release_directive( release_directive_statement = ( "set default release directive" - if same_identifiers(release_directive, "DEFAULT") + if same_identifiers(release_directive, DEFAULT_DIRECTIVE) else f"set release directive {release_directive}" ) @@ -974,7 +978,7 @@ def modify_release_directive( release_directive_statement = ( "modify default release directive" - if same_identifiers(release_directive, "DEFAULT") + if same_identifiers(release_directive, DEFAULT_DIRECTIVE) else f"modify release directive {release_directive}" ) @@ -1034,7 +1038,7 @@ def unset_release_directive( release_channel = to_identifier(release_channel) if release_channel else None release_directive = to_identifier(release_directive) - if same_identifiers(release_directive, "DEFAULT"): + if same_identifiers(release_directive, DEFAULT_DIRECTIVE): raise UserInputError( "Cannot unset default release directive. Please specify a non-default release directive." ) diff --git a/tests/nativeapp/factories.py b/tests/nativeapp/factories.py index d0c67f0601..8a4c51c5e7 100644 --- a/tests/nativeapp/factories.py +++ b/tests/nativeapp/factories.py @@ -355,7 +355,6 @@ class ManifestFactory(factory.DictFactory): { "log_level": "fatal", "trace_level": "always", - "telemetry_event_definitions": None, } )