Add package scripts warning back (#1764) (#1784)

snowflakedb · Oct 23, 2024 · 8a6b874 · 8a6b874
1 parent 5f72387
commit 8a6b874
Show file tree

Hide file tree

Showing 6 changed files with 11 additions and 77 deletions.
diff --git a/src/snowflake/cli/_plugins/nativeapp/entities/application_package.py b/src/snowflake/cli/_plugins/nativeapp/entities/application_package.py
@@ -51,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,
 )
@@ -76,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
@@ -212,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,
         )
 
@@ -322,7 +319,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
             version=version,
             patch=patch,
             skip_git_check=skip_git_check,
@@ -402,7 +398,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
@@ -427,16 +422,8 @@ def deploy(
             console.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):
-            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)
             diff = sync_deploy_root_with_stage(
@@ -520,7 +507,6 @@ def version_create(
         validate: bool,
         stage_fqn: str,
         post_deploy_hooks: list[PostDeployHook] | None,
-        package_scripts: List[str],
         version: Optional[str],
         patch: Optional[int],
         force: bool,
@@ -613,7 +599,6 @@ def version_create(
             stage_fqn=stage_fqn,
             package_warehouse=package_warehouse,
             post_deploy_hooks=post_deploy_hooks,
-            package_scripts=package_scripts,
             policy=policy,
         )
 
@@ -1054,51 +1039,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
-        with cls.use_package_warehouse(
-            package_warehouse=package_warehouse,
-        ):
-            try:
-                for i, queries in enumerate(queued_queries):
-                    console.step(f"Applying package script: {package_scripts[i]}")
-                    get_sql_executor().execute_queries(queries)
-            except ProgrammingError as err:
-                generic_sql_error_handler(err)
-
     @classmethod
     def create_app_package(
         cls,
@@ -1308,7 +1248,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/_plugins/nativeapp/manager.py b/src/snowflake/cli/_plugins/nativeapp/manager.py
@@ -256,16 +256,6 @@ def create_app_package(self) -> None:
             package_distribution=self.package_distribution,
         )
 
-    def _apply_package_scripts(self) -> None:
-        return ApplicationPackageEntity.apply_package_scripts(
-            console=cc,
-            package_scripts=self.package_scripts,
-            package_warehouse=self.package_warehouse,
-            project_root=self.project_root,
-            package_role=self.package_role,
-            package_name=self.package_name,
-        )
-
     def execute_app_post_deploy_hooks(self) -> None:
         execute_post_deploy_hooks(
             console=cc,

diff --git 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
@@ -1382,7 +1382,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
@@ -1471,7 +1470,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

diff --git a/tests/nativeapp/test_version_create.py b/tests/nativeapp/test_version_create.py
@@ -79,7 +79,6 @@ def _version_create(
         stage_fqn=f"{pkg_model.fqn.name}.{pkg_model.stage}",
         package_warehouse=pkg_model.meta.warehouse,
         post_deploy_hooks=pkg_model.meta.post_deploy,
-        package_scripts=[],
         version=version,
         patch=patch,
         force=force,

diff --git 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,
             },
         ),
     ],