Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

refactor post deploy hooks to use sql facade and clean up tests with factories #1804

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

refactor post deploy hooks to use sql facade and clean up tests with factories #1804

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8d958ec
refactor post deploy hooks to use sql facade and clean up tests with …
sfc-gh-mchok Oct 25, 2024
f47de68
Merge branch 'main' into mchok-migrate-post-deploy-scripts-to-sqlfacade
sfc-gh-mchok Oct 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 12 additions & 17 deletions src/snowflake/cli/_plugins/nativeapp/entities/application.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@
from snowflake.cli._plugins.nativeapp.sf_facade import get_snowflake_facade
from snowflake.cli._plugins.nativeapp.utils import needs_confirmation
from snowflake.cli._plugins.workspace.context import ActionContext
from snowflake.cli.api.cli_global_context import get_cli_context
from snowflake.cli.api.entities.common import EntityBase, get_sql_executor
from snowflake.cli.api.entities.utils import (
drop_generic_object,
Expand All @@ -60,7 +59,6 @@
NOT_SUPPORTED_ON_DEV_MODE_APPLICATIONS,
ONLY_SUPPORTED_ON_DEV_MODE_APPLICATIONS,
)
from snowflake.cli.api.metrics import CLICounterField
from snowflake.cli.api.project.schemas.entities.common import (
EntityModelBase,
Identifier,
Expand Down Expand Up @@ -447,18 +445,15 @@ def create_or_upgrade_app(
model = self._entity_model
workspace_ctx = self._workspace_ctx
console = workspace_ctx.console
project_root = workspace_ctx.project_root

app_name = model.fqn.identifier
debug_mode = model.debug
if model.meta:
app_role = model.meta.role or workspace_ctx.default_role
app_warehouse = model.meta.warehouse or workspace_ctx.default_warehouse
post_deploy_hooks = model.meta.post_deploy
else:
app_role = workspace_ctx.default_role
app_warehouse = workspace_ctx.default_warehouse
post_deploy_hooks = None

package_name = package_model.fqn.identifier
if package_model.meta and package_model.meta.role:
Expand Down Expand Up @@ -569,23 +564,23 @@ def execute_post_deploy_hooks(self):
workspace_ctx = self._workspace_ctx
console = workspace_ctx.console
project_root = workspace_ctx.project_root
app_role = (model.meta and model.meta.role) or workspace_ctx.default_role
app_warehouse = (
model.meta and model.meta.warehouse
) or workspace_ctx.default_warehouse
Comment on lines +567 to +570
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you should rebase on top of Frank's latest work (expect conflicts though, but it will make your job easier overall)

app_name = model.fqn.identifier
post_deploy_hooks = model.meta and model.meta.post_deploy

get_cli_context().metrics.set_counter_default(
CLICounterField.POST_DEPLOY_SCRIPTS, 0
execute_post_deploy_hooks(
console=console,
project_root=project_root,
post_deploy_hooks=post_deploy_hooks,
deployed_object_type="application",
role_name=app_role,
warehouse_name=app_warehouse,
database_name=app_name,
)

if post_deploy_hooks:
with self.use_application_warehouse():
execute_post_deploy_hooks(
console=console,
project_root=project_root,
post_deploy_hooks=post_deploy_hooks,
deployed_object_type="application",
database_name=app_name,
)

@contextmanager
def use_application_warehouse(self):
model = self._entity_model
Expand Down
52 changes: 14 additions & 38 deletions src/snowflake/cli/_plugins/nativeapp/entities/application_package.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
from __future__ import annotations

import json
from contextlib import contextmanager
from pathlib import Path
from textwrap import dedent
from typing import List, Literal, Optional, Union
Expand Down Expand Up @@ -44,7 +43,6 @@
from snowflake.cli._plugins.stage.diff import DiffResult
from snowflake.cli._plugins.stage.manager import StageManager
from snowflake.cli._plugins.workspace.context import ActionContext
from snowflake.cli.api.cli_global_context import get_cli_context
from snowflake.cli.api.entities.common import EntityBase, get_sql_executor
from snowflake.cli.api.entities.utils import (
drop_generic_object,
Expand All @@ -55,7 +53,6 @@
)
from snowflake.cli.api.errno import DOES_NOT_EXIST_OR_NOT_AUTHORIZED
from snowflake.cli.api.exceptions import SnowflakeSQLExecutionError
from snowflake.cli.api.metrics import CLICounterField
from snowflake.cli.api.project.schemas.entities.common import (
EntityModelBase,
Identifier,
Expand Down Expand Up @@ -596,8 +593,8 @@ def _deploy(
print_diff=print_diff,
)

if run_post_deploy_hooks:
self.execute_post_deploy_hooks()
if run_post_deploy_hooks:
self.execute_post_deploy_hooks()
Comment on lines +596 to +597
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this will be calling use_role through the sql facade, we don't need to nest it inside the above context manager


if validate:
self.validate_setup_script(
Expand Down Expand Up @@ -859,27 +856,6 @@ def verify_project_distribution(
return False
return True

@contextmanager
def use_package_warehouse(self):
model = self._entity_model
ctx = self._workspace_ctx
package_warehouse = (
model.meta and model.meta.warehouse and to_identifier(model.meta.warehouse)
) or to_identifier(ctx.default_warehouse)

if package_warehouse:
with get_sql_executor().use_warehouse(package_warehouse):
yield
else:
raise ClickException(
dedent(
f"""\
Application package warehouse cannot be empty.
Please provide a value for it in your connection information or your project definition file.
"""
)
)

def create_app_package(self) -> None:
"""
Creates the application package with our up-to-date stage if none exists.
Expand Down Expand Up @@ -932,23 +908,23 @@ def execute_post_deploy_hooks(self):
workspace_ctx = self._workspace_ctx
console = workspace_ctx.console
project_root = workspace_ctx.project_root
package_role = (model.meta and model.meta.role) or workspace_ctx.default_role
package_warehouse = (
model.meta and model.meta.warehouse
) or workspace_ctx.default_warehouse
package_name = model.fqn.identifier
post_deploy_hooks = model.meta and model.meta.post_deploy

get_cli_context().metrics.set_counter_default(
CLICounterField.POST_DEPLOY_SCRIPTS, 0
execute_post_deploy_hooks(
console=console,
project_root=project_root,
post_deploy_hooks=post_deploy_hooks,
deployed_object_type="application package",
role_name=package_role,
warehouse_name=package_warehouse,
database_name=package_name,
)

if post_deploy_hooks:
with self.use_package_warehouse():
execute_post_deploy_hooks(
console=console,
project_root=project_root,
post_deploy_hooks=post_deploy_hooks,
deployed_object_type="application package",
database_name=package_name,
)

def validate_setup_script(
self, use_scratch_stage: bool, interactive: bool, force: bool
):
Expand Down
36 changes: 15 additions & 21 deletions src/snowflake/cli/api/entities/utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
InvalidTemplateInFileError,
MissingScriptError,
)
from snowflake.cli._plugins.nativeapp.sf_facade import get_snowflake_facade
from snowflake.cli._plugins.nativeapp.utils import verify_exists, verify_no_directories
from snowflake.cli._plugins.stage.diff import (
DiffResult,
Expand Down Expand Up @@ -195,36 +196,24 @@ def sync_deploy_root_with_stage(
return diff


def _execute_sql_script(
script_content: str,
database_name: Optional[str] = None,
) -> None:
"""
Executing the provided SQL script content.
This assumes that a relevant warehouse is already active.
If database_name is passed in, it will be used first.
"""
try:
sql_executor = get_sql_executor()
if database_name is not None:
sql_executor.execute_query(f"use database {database_name}")
sql_executor.execute_queries(script_content)
except ProgrammingError as err:
generic_sql_error_handler(err)


def execute_post_deploy_hooks(
console: AbstractConsole,
project_root: Path,
post_deploy_hooks: Optional[List[PostDeployHook]],
deployed_object_type: str,
role_name: str,
database_name: str,
warehouse_name: str,
) -> None:
"""
Executes post-deploy hooks for the given object type.
While executing SQL post deploy hooks, it first switches to the database provided in the input.
All post deploy scripts templates will first be expanded using the global template context.
"""
get_cli_context().metrics.set_counter_default(
CLICounterField.POST_DEPLOY_SCRIPTS, 0
)

if not post_deploy_hooks:
return

Expand All @@ -248,11 +237,16 @@ def execute_post_deploy_hooks(
sql_scripts_paths,
)

sql_facade = get_snowflake_facade()

for index, sql_script_path in enumerate(display_paths):
console.step(f"Executing SQL script: {sql_script_path}")
_execute_sql_script(
script_content=scripts_content_list[index],
database_name=database_name,
sql_facade.execute_user_script(
queries=scripts_content_list[index],
script_name=sql_script_path,
role=role_name,
warehouse=warehouse_name,
database=database_name,
)


Expand Down
Loading
Loading