Address comments

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.

src/snowflake/cli/_plugins/nativeapp/constants.py

@@ -27,3 +27,6 @@
 
 INTERNAL_DISTRIBUTION = "internal"
 EXTERNAL_DISTRIBUTION = "external"
+
+DEFAULT_CHANNEL = "DEFAULT"
+DEFAULT_DIRECTIVE = "DEFAULT"

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,26 +672,26 @@ 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}."
             )
 
         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.",

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,

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."
             )

tests/nativeapp/factories.py

@@ -355,7 +355,6 @@ class ManifestFactory(factory.DictFactory):
         {
             "log_level": "fatal",
             "trace_level": "always",
-            "telemetry_event_definitions": None,
         }
     )