Skip to content

Commit

Permalink
Catch error on deploy for package with special comment (#1600)
Browse files Browse the repository at this point in the history
  • Loading branch information
@sfc-gh-pjafari
sfc-gh-pjafari committed Sep 20, 2024
1 parent 0dd7426 commit 9176f5b
Show file tree
Hide file tree
Showing 12 changed files with 327 additions and 52 deletions.
1 change: 1 addition & 0 deletions RELEASE-NOTES.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@
* `snow streamlit deploy` will check for existing streamlit instance before deploying anything.
* Fixed `snow git execute` with `/` in name of the branch.
* `snow app` commands don't enforce ownership of the objects they manage, and rely on RBAC instead.
* `snow app deploy` for package entity now allows operating on application packages created outside the CLI

# v2.8.1
## Backward incompatibility
Expand Down
2 changes: 2 additions & 0 deletions src/snowflake/cli/_plugins/nativeapp/application_entity.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,8 @@ def deploy_package():
paths=[],
validate=validate,
stage_fqn=stage_fqn,
interactive=interactive,
force=force,
)

self.deploy(
Expand Down
43 changes: 35 additions & 8 deletions src/snowflake/cli/_plugins/nativeapp/application_package_entity.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,19 @@
CouldNotDropApplicationPackageWithVersions,
SetupScriptFailedValidation,
)
from snowflake.cli._plugins.nativeapp.policy import (
AllowAlwaysPolicy,
AskAlwaysPolicy,
DenyAlwaysPolicy,
PolicyBase,
)
from snowflake.cli._plugins.nativeapp.utils import (
needs_confirmation,
)
from snowflake.cli._plugins.stage.diff import DiffResult
from snowflake.cli._plugins.stage.manager import StageManager
from snowflake.cli._plugins.workspace.action_context import ActionContext
from snowflake.cli.api.console import cli_console as cc
from snowflake.cli.api.console.abc import AbstractConsole
from snowflake.cli.api.entities.common import EntityBase, get_sql_executor
from snowflake.cli.api.entities.utils import (
Expand Down Expand Up @@ -80,12 +87,22 @@ def action_deploy(
recursive: bool,
paths: List[Path],
validate: bool,
interactive: bool,
force: bool,
stage_fqn: Optional[str] = None,
*args,
**kwargs,
):
model = self._entity_model
package_name = model.fqn.identifier

if force:
policy = AllowAlwaysPolicy()
elif interactive:
policy = AskAlwaysPolicy()
else:
policy = DenyAlwaysPolicy()

return self.deploy(
console=ctx.console,
project_root=ctx.project_root,
Expand All @@ -107,6 +124,7 @@ def action_deploy(
),
post_deploy_hooks=model.meta and model.meta.post_deploy,
package_scripts=[], # Package scripts are not supported in PDFv2
policy=policy,
)

def action_drop(self, ctx: ActionContext, force_drop: bool, *args, **kwargs):
Expand All @@ -124,7 +142,9 @@ def action_drop(self, ctx: ActionContext, force_drop: bool, *args, **kwargs):
force_drop=force_drop,
)

def action_validate(self, ctx: ActionContext, *args, **kwargs):
def action_validate(
self, ctx: ActionContext, interactive: bool, force: bool, *args, **kwargs
):
model = self._entity_model
package_name = model.fqn.identifier
stage_fqn = f"{package_name}.{model.stage}"
Expand All @@ -141,6 +161,8 @@ def deploy_to_scratch_stage_fn():
paths=[],
validate=False,
stage_fqn=model.scratch_stage,
interactive=interactive,
force=force,
)

self.validate_setup_script(
Expand Down Expand Up @@ -206,6 +228,7 @@ def deploy(
stage_fqn: str,
post_deploy_hooks: list[PostDeployHook] | None,
package_scripts: List[str],
policy: PolicyBase,
) -> DiffResult:
# 1. Create a bundle
bundle_map = cls.bundle(
Expand All @@ -218,13 +241,17 @@ def deploy(
)

# 2. Create an empty application package, if none exists
cls.create_app_package(
console=console,
package_name=package_name,
package_role=package_role,
package_distribution=package_distribution,
)

try:
cls.create_app_package(
console=console,
package_name=package_name,
package_role=package_role,
package_distribution=package_distribution,
)
except ApplicationPackageAlreadyExistsError as e:
cc.warning(e.message)
if not policy.should_proceed("Proceed with using this package?"):
raise typer.Abort() from e
with get_sql_executor().use_role(package_role):
if package_scripts:
cls.apply_package_scripts(
Expand Down
14 changes: 13 additions & 1 deletion src/snowflake/cli/_plugins/nativeapp/commands.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -406,6 +406,8 @@ def app_deploy(
unspecified, the command syncs all local changes to the stage."""
).strip(),
),
interactive: bool = InteractiveOption,
force: Optional[bool] = ForceOption,
validate: bool = ValidateOption,
**options,
) -> CommandResult:
Expand All @@ -416,6 +418,13 @@ def app_deploy(

assert_project_type("native_app")

if force:
policy = AllowAlwaysPolicy()
elif interactive:
policy = AskAlwaysPolicy()
else:
policy = DenyAlwaysPolicy()

has_paths = paths is not None and len(paths) > 0
if prune is None and recursive is None and not has_paths:
prune = True
Expand All @@ -442,6 +451,7 @@ def app_deploy(
recursive=recursive,
local_paths_to_sync=paths,
validate=validate,
policy=policy,
)

return MessageResult(
Expand All @@ -452,7 +462,9 @@ def app_deploy(
@app.command("validate", requires_connection=True)
@with_project_definition()
@nativeapp_definition_v2_to_v1()
def app_validate(**options):
def app_validate(
**options,
):
"""
Validates a deployed Snowflake Native App's setup script.
"""
Expand Down
4 changes: 4 additions & 0 deletions src/snowflake/cli/_plugins/nativeapp/manager.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
from snowflake.cli._plugins.nativeapp.exceptions import (
NoEventTableForAccount,
)
from snowflake.cli._plugins.nativeapp.policy import AllowAlwaysPolicy, PolicyBase
from snowflake.cli._plugins.nativeapp.project_model import (
NativeAppProjectModel,
)
Expand Down Expand Up @@ -306,6 +307,7 @@ def deploy(
bundle_map: BundleMap,
prune: bool,
recursive: bool,
policy: PolicyBase,
stage_fqn: Optional[str] = None,
local_paths_to_sync: List[Path] | None = None,
validate: bool = True,
Expand All @@ -330,6 +332,7 @@ def deploy(
package_warehouse=self.package_warehouse,
post_deploy_hooks=self.package_post_deploy_hooks,
package_scripts=self.package_scripts,
policy=policy,
)

def deploy_to_scratch_stage_fn(self):
Expand All @@ -341,6 +344,7 @@ def deploy_to_scratch_stage_fn(self):
stage_fqn=self.scratch_stage_fqn,
validate=False,
print_diff=False,
policy=AllowAlwaysPolicy(),
)

def validate(self, use_scratch_stage: bool = False):
Expand Down
3 changes: 3 additions & 0 deletions src/snowflake/cli/_plugins/nativeapp/policy.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ class PolicyBase(ABC):
def should_proceed(self, user_prompt: Optional[str]) -> bool:
pass

def __eq__(self, value: object) -> bool:
return self.__class__ == value.__class__


class AllowAlwaysPolicy(PolicyBase):
"""Always allow a Snowflake CLI command to continue execution."""
Expand Down
6 changes: 5 additions & 1 deletion src/snowflake/cli/_plugins/nativeapp/run_processor.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,11 @@ def process(
):
def deploy_package():
self.deploy(
bundle_map=bundle_map, prune=True, recursive=True, validate=validate
bundle_map=bundle_map,
prune=True,
recursive=True,
validate=validate,
policy=policy,
)

def drop_app():
Expand Down
4 changes: 4 additions & 0 deletions src/snowflake/cli/_plugins/workspace/commands.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,8 @@ def validate(
entity_id: str = typer.Option(
help=f"""The ID of the entity you want to validate.""",
),
interactive: bool = InteractiveOption,
force: Optional[bool] = ForceOption,
**options,
):
"""Validates the specified entity."""
Expand All @@ -191,6 +193,8 @@ def validate(
ws.perform_action(
entity_id,
EntityActions.VALIDATE,
interactive=interactive,
force=force,
)


Expand Down
105 changes: 64 additions & 41 deletions tests/__snapshots__/test_help_messages.ambr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,47 +96,70 @@
| syncs all local changes to the stage. |
+------------------------------------------------------------------------------+
+- Options --------------------------------------------------------------------+
| --prune --no-prune Whether to delete specified |
| files from the stage if |
| they don't exist locally. |
| If set, the command deletes |
| files that exist in the |
| stage, but not in the local |
| filesystem. This option |
| cannot be used when paths |
| are specified. |
| [default: no-prune] |
| --recursive -r --no-recursive Whether to traverse and |
| deploy files from |
| subdirectories. If set, the |
| command deploys all files |
| and subdirectories; |
| otherwise, only files in |
| the current directory are |
| deployed. |
| [default: no-recursive] |
| --validate --no-validate When enabled, this option |
| triggers validation of a |
| deployed Snowflake Native |
| App's setup script SQL |
| [default: validate] |
| --package-entity-id TEXT The ID of the package |
| entity on which to operate |
| when definition_version is |
| 2 or higher. |
| --app-entity-id TEXT The ID of the application |
| entity on which to operate |
| when definition_version is |
| 2 or higher. |
| --project -p TEXT Path where Snowflake |
| project resides. Defaults |
| to current working |
| directory. |
| --env TEXT String in format of |
| key=value. Overrides |
| variables from env section |
| used for templates. |
| --help -h Show this message and exit. |
| --prune --no-prune Whether to delete |
| specified files from the |
| stage if they don't exist |
| locally. If set, the |
| command deletes files |
| that exist in the stage, |
| but not in the local |
| filesystem. This option |
| cannot be used when paths |
| are specified. |
| [default: no-prune] |
| --recursive -r --no-recursive Whether to traverse and |
| deploy files from |
| subdirectories. If set, |
| the command deploys all |
| files and subdirectories; |
| otherwise, only files in |
| the current directory are |
| deployed. |
| [default: no-recursive] |
| --interactive --no-interactive When enabled, this option |
| displays prompts even if |
| the standard input and |
| output are not terminal |
| devices. Defaults to True |
| in an interactive shell |
| environment, and False |
| otherwise. |
| --force When enabled, this option |
| causes the command to |
| implicitly approve any |
| prompts that arise. You |
| should enable this option |
| if interactive mode is |
| not specified and if you |
| want perform potentially |
| destructive actions. |
| Defaults to unset. |
| --validate --no-validate When enabled, this option |
| triggers validation of a |
| deployed Snowflake Native |
| App's setup script SQL |
| [default: validate] |
| --package-entity-id TEXT The ID of the package |
| entity on which to |
| operate when |
| definition_version is 2 |
| or higher. |
| --app-entity-id TEXT The ID of the application |
| entity on which to |
| operate when |
| definition_version is 2 |
| or higher. |
| --project -p TEXT Path where Snowflake |
| project resides. Defaults |
| to current working |
| directory. |
| --env TEXT String in format of |
| key=value. Overrides |
| variables from env |
| section used for |
| templates. |
| --help -h Show this message and |
| exit. |
+------------------------------------------------------------------------------+
+- Connection configuration ---------------------------------------------------+
| --connection,--environment -c TEXT Name of the connection, as |
Expand Down
Loading

0 comments on commit 9176f5b

Please sign in to comment.