From 156c155cefe4f4a1f630b9b65e75e189e335ef2c Mon Sep 17 00:00:00 2001 From: Michel El Nacouzi Date: Tue, 22 Oct 2024 10:25:33 -0400 Subject: [PATCH 1/2] Add package scripts warning back --- .../nativeapp/entities/application_package.py | 60 ------------------- .../cli/api/project/definition_conversion.py | 9 +++ tests/nativeapp/test_manager.py | 2 - 3 files changed, 9 insertions(+), 62 deletions(-) diff --git a/src/snowflake/cli/_plugins/nativeapp/entities/application_package.py b/src/snowflake/cli/_plugins/nativeapp/entities/application_package.py index dffb3b3ded..b8df93ef03 100644 --- a/src/snowflake/cli/_plugins/nativeapp/entities/application_package.py +++ b/src/snowflake/cli/_plugins/nativeapp/entities/application_package.py @@ -40,7 +40,6 @@ DenyAlwaysPolicy, PolicyBase, ) -from snowflake.cli._plugins.nativeapp.sf_facade import get_snowflake_facade 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 @@ -52,7 +51,6 @@ drop_generic_object, execute_post_deploy_hooks, generic_sql_error_handler, - render_script_templates, sync_deploy_root_with_stage, validation_item_to_str, ) @@ -77,7 +75,6 @@ to_identifier, unquote_identifier, ) -from snowflake.cli.api.rendering.jinja import get_basic_jinja_env from snowflake.cli.api.utils.cursor import find_all_rows from snowflake.connector import DictCursor, ProgrammingError from snowflake.connector.cursor import SnowflakeCursor @@ -213,7 +210,6 @@ def action_deploy( (model.meta and model.meta.warehouse) or workspace_ctx.default_warehouse ), post_deploy_hooks=model.meta and model.meta.post_deploy, - package_scripts=[], # Package scripts are not supported in PDFv2 policy=policy, ) @@ -493,7 +489,6 @@ def action_version_create( (model.meta and model.meta.warehouse) or workspace_ctx.default_warehouse ), post_deploy_hooks=(model.meta and model.meta.post_deploy), - package_scripts=[], # Package scripts are not supported in PDFv2 policy=policy, ) @@ -623,7 +618,6 @@ def deploy( validate: bool, stage_fqn: str, post_deploy_hooks: list[PostDeployHook] | None, - package_scripts: List[str], policy: PolicyBase, ) -> DiffResult: # 1. Create a bundle if one wasn't passed in @@ -649,14 +643,6 @@ def deploy( if not policy.should_proceed("Proceed with using this package?"): raise typer.Abort() from e - cls.apply_package_scripts( - console=console, - package_scripts=package_scripts, - package_warehouse=package_warehouse, - project_root=project_root, - package_role=package_role, - package_name=package_name, - ) with get_sql_executor().use_role(package_role): # 3. Upload files from deploy root local folder to the above stage stage_schema = extract_schema(stage_fqn) @@ -1088,51 +1074,6 @@ def use_package_warehouse( ) ) - @classmethod - def apply_package_scripts( - cls, - console: AbstractConsole, - package_scripts: List[str], - package_warehouse: Optional[str], - project_root: Path, - package_role: str, - package_name: str, - ) -> None: - """ - Assuming the application package exists and we are using the correct role, - applies all package scripts in-order to the application package. - """ - - metrics = get_cli_context().metrics - metrics.set_counter_default(CLICounterField.PACKAGE_SCRIPTS, 0) - - if not package_scripts: - return - - metrics.set_counter(CLICounterField.PACKAGE_SCRIPTS, 1) - - console.warning( - "WARNING: native_app.package.scripts is deprecated. Please migrate to using native_app.package.post_deploy." - ) - - queued_queries = render_script_templates( - project_root, - dict(package_name=package_name), - package_scripts, - get_basic_jinja_env(), - ) - - # once we're sure all the templates expanded correctly, execute all of them - for i, queries in enumerate(queued_queries): - script_name = package_scripts[i] - console.step(f"Applying package script: {script_name}") - get_snowflake_facade().execute_user_script( - queries=queries, - script_name=script_name, - role=package_role, - warehouse=package_warehouse, - ) - @classmethod def create_app_package( cls, @@ -1342,7 +1283,6 @@ def get_validation_result_static( stage_fqn=stage_fqn, package_warehouse=package_warehouse, post_deploy_hooks=[], - package_scripts=[], policy=policy, ) prefixed_stage_fqn = StageManager.get_standard_stage_prefix(stage_fqn) diff --git a/src/snowflake/cli/api/project/definition_conversion.py b/src/snowflake/cli/api/project/definition_conversion.py index f5e7f27706..78c5373f42 100644 --- a/src/snowflake/cli/api/project/definition_conversion.py +++ b/src/snowflake/cli/api/project/definition_conversion.py @@ -13,6 +13,8 @@ ApplicationPackageEntityModel, ) from snowflake.cli._plugins.snowpark.common import is_name_a_templated_one +from snowflake.cli.api.cli_global_context import get_cli_context +from snowflake.cli.api.console import cli_console from snowflake.cli.api.constants import ( DEFAULT_ENV_FILE, DEFAULT_PAGES_DIR, @@ -21,6 +23,7 @@ SNOWPARK_SHARED_MIXIN, ) from snowflake.cli.api.entities.utils import render_script_template +from snowflake.cli.api.metrics import CLICounterField from snowflake.cli.api.project.schemas.entities.common import ( MetaField, SqlScriptHookType, @@ -381,8 +384,14 @@ def _convert_templates_in_files( """Converts templates in other files to the new format""" # TODO handle artifacts using the "templates" processor # For now this only handles Native App package scripts + metrics = get_cli_context().metrics + metrics.set_counter_default(CLICounterField.PACKAGE_SCRIPTS, 0) if (na := definition_v1.native_app) and (pkg := na.package) and pkg.scripts: + metrics.set_counter(CLICounterField.PACKAGE_SCRIPTS, 1) + cli_console.warning( + "WARNING: native_app.package.scripts is deprecated. Please migrate to using native_app.package.post_deploy." + ) # If the v1 definition has a Native App with a package, we know # that the v2 definition will have exactly one application package entity pkg_entity: ApplicationPackageEntityModel = list( diff --git a/tests/nativeapp/test_manager.py b/tests/nativeapp/test_manager.py index 40dae65a10..98b4b32fba 100644 --- a/tests/nativeapp/test_manager.py +++ b/tests/nativeapp/test_manager.py @@ -1375,7 +1375,6 @@ def test_validate_use_scratch_stage(mock_execute, mock_deploy, temp_dir, mock_cu stage_fqn=f"{pkg_model.fqn.name}.{pkg_model.scratch_stage}", package_warehouse=pkg_model.meta.warehouse, post_deploy_hooks=[], - package_scripts=[], policy=AllowAlwaysPolicy(), ) assert mock_execute.mock_calls == expected @@ -1464,7 +1463,6 @@ def test_validate_failing_drops_scratch_stage( stage_fqn=f"{pkg_model.fqn.name}.{pkg_model.scratch_stage}", package_warehouse=pkg_model.meta.warehouse, post_deploy_hooks=[], - package_scripts=[], policy=AllowAlwaysPolicy(), ) assert mock_execute.mock_calls == expected From a0960472109f2705d4f1def291262eb38db25f0d Mon Sep 17 00:00:00 2001 From: Michel El Nacouzi Date: Tue, 22 Oct 2024 11:35:06 -0400 Subject: [PATCH 2/2] Fix integration tests for emitting package scripts metric --- tests_integration/nativeapp/test_feature_metrics.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests_integration/nativeapp/test_feature_metrics.py b/tests_integration/nativeapp/test_feature_metrics.py index 618c59ceb7..5ea61226c9 100644 --- a/tests_integration/nativeapp/test_feature_metrics.py +++ b/tests_integration/nativeapp/test_feature_metrics.py @@ -103,7 +103,7 @@ def test_sql_templating_emits_counter( CLICounterField.TEMPLATES_PROCESSOR: 0, CLICounterField.PDF_TEMPLATES: 1, CLICounterField.POST_DEPLOY_SCRIPTS: 1, - CLICounterField.PACKAGE_SCRIPTS: 0, + CLICounterField.PACKAGE_SCRIPTS: 1, }, ), # ensure post deploy scripts are picked up for v2 @@ -115,7 +115,6 @@ def test_sql_templating_emits_counter( CLICounterField.TEMPLATES_PROCESSOR: 0, CLICounterField.PDF_TEMPLATES: 1, CLICounterField.POST_DEPLOY_SCRIPTS: 1, - CLICounterField.PACKAGE_SCRIPTS: 0, }, ), ],