Added additional check for new imports (#900)

* Added additional check for new imports

fixed test

Added check for imports

Test and release notes

* Post review fixes

* Post review fixes

* Post review fixes

* Post Post review fixes

* Post Post review fixes

* Post Post review fixes

RELEASE-NOTES.md

@@ -17,6 +17,7 @@
 * Adding `--image-name` option for image name argument in `spcs image-repository list-tags` for consistency with other commands.
 * Fixed errors during `spcs image-registry login` not being formatted correctly.
 * Project definition no longer accept extra fields. Any extra field will cause an error.
+* Changing imports in function/procedure section in `snowflake.yml` will cause the definition update on replace
 * Adding `--pattern` flag to `stage list` command for filtering out results with regex.
 
 # v2.1.0

src/snowflake/cli/plugins/snowpark/commands.py

@@ -143,6 +143,7 @@ def deploy(
             existing_objects=existing_procedures,
             packages=packages,
             stage_artifact_path=artifact_stage_target,
+            source_name=build_artifact_path.name,
         )
         deploy_status.append(operation_result)
 
@@ -155,6 +156,7 @@ def deploy(
             existing_objects=existing_functions,
             packages=packages,
             stage_artifact_path=artifact_stage_target,
+            source_name=build_artifact_path.name,
         )
         deploy_status.append(operation_result)
 
@@ -241,6 +243,7 @@ def _deploy_single_object(
     existing_objects: Dict[str, Dict],
     packages: List[str],
     stage_artifact_path: str,
+    source_name: str,
 ):
     identifier = build_udf_sproc_identifier(
         object_definition, manager, include_parameter_names=False
@@ -255,6 +258,7 @@ def _deploy_single_object(
 
     handler = object_definition.handler
     returns = object_definition.returns
+    imports = object_definition.imports
     replace_object = False
 
     object_exists = identifier in existing_objects
@@ -264,6 +268,8 @@ def _deploy_single_object(
             existing_objects[identifier],
             handler,
             returns,
+            imports,
+            stage_artifact_path,
         )
 
     if object_exists and not replace_object:
@@ -282,7 +288,7 @@ def _deploy_single_object(
         "runtime": object_definition.runtime,
         "external_access_integrations": object_definition.external_access_integrations,
         "secrets": object_definition.secrets,
-        "imports": object_definition.imports,
+        "imports": imports,
     }
     if object_type == ObjectType.PROCEDURE:
         create_or_replace_kwargs[

src/snowflake/cli/plugins/snowpark/common.py

@@ -1,5 +1,6 @@
 from __future__ import annotations
 
+import re
 from typing import Dict, List, Optional
 
 from snowflake.cli.api.constants import DEFAULT_SIZE_LIMIT_MB, ObjectType
@@ -17,6 +18,8 @@ def check_if_replace_is_required(
     current_state,
     handler: str,
     return_type: str,
+    imports: List[str],
+    stage_artifact_file: str,
 ) -> bool:
     import logging
 
@@ -50,6 +53,9 @@ def check_if_replace_is_required(
         )
         return True
 
+    if _compare_imports(resource_json, imports, stage_artifact_file):
+        return True
+
     return False
 
 
@@ -197,3 +203,25 @@ def format_arg(arg: Argument):
         schema=udf_sproc.schema_name,
     )
     return f"{name}({arguments})"
+
+
+def _compare_imports(
+    resource_json: dict, imports: List[str], artifact_file: str
+) -> bool:
+    pattern = re.compile(r"(?:\[@?\w+_\w+\.)?(\w+(?:/\w+)+\.\w+)(?:\])?")
+
+    project_imports = {
+        imp
+        for import_string in [*imports, artifact_file]
+        for imp in pattern.findall(import_string.lower())
+    }
+
+    if "imports" not in resource_json.keys():
+        object_imports = set()
+    else:
+        object_imports = {
+            imp.lower()
+            for imp in pattern.findall(resource_json.get("imports", "").lower())
+        }
+
+    return project_imports != object_imports

tests/snowpark/test_function.py

@@ -138,6 +138,7 @@ def test_deploy_function_no_changes(
         ("packages", '["foo=1.2.3", "bar>=3.0.0"]'),
         ("handler", "app.func1_handler"),
         ("returns", "string"),
+        ("imports", "DEV_DEPLOYMENT/my_snowpark_project/app.zip"),
     ]
 
     queries, result, project_dir = _deploy_function(

tests/snowpark/test_procedure.py

@@ -254,6 +254,7 @@ def test_deploy_procedure_replace_nothing_to_update(
                 ("packages", "[]"),
                 ("handler", "hello"),
                 ("returns", "string"),
+                ("imports", "DEV_DEPLOYMENT/my_snowpark_project/app.zip"),
             ],
             columns=["key", "value"],
         ),
@@ -262,6 +263,7 @@ def test_deploy_procedure_replace_nothing_to_update(
                 ("packages", "[]"),
                 ("handler", "test"),
                 ("returns", "string"),
+                ("imports", "DEV_DEPLOYMENT/my_snowpark_project/app.zip"),
             ],
             columns=["key", "value"],
         ),
@@ -305,6 +307,7 @@ def test_deploy_procedure_replace_updates_single_object(
                 ("packages", "[]"),
                 ("handler", "hello"),
                 ("returns", "string"),
+                ("imports", "DEV_DEPLOYMENT/my_snowpark_project/app.zip"),
             ],
             columns=["key", "value"],
         ),
@@ -313,6 +316,7 @@ def test_deploy_procedure_replace_updates_single_object(
                 ("packages", "[]"),
                 ("handler", "foo"),
                 ("returns", "string"),
+                ("imports", "DEV_DEPLOYMENT/my_snowpark_project/app.zip"),
             ],
             columns=["key", "value"],
         ),
@@ -356,6 +360,7 @@ def test_deploy_procedure_replace_creates_missing_object(
                 ("packages", "[]"),
                 ("handler", "hello"),
                 ("returns", "string"),
+                ("imports", "DEV_DEPLOYMENT/my_snowpark_project/app.zip"),
             ],
             columns=["key", "value"],
         ),

tests/testing_utils/fixtures.py

@@ -296,7 +296,10 @@ def _update(snowflake_yml_path: Path, parameter_path: str, value=None):
             evaluated_part = int(part) if part.isdigit() else part
 
             if parts:
-                current_object = current_object[evaluated_part]
+                if isinstance(current_object, dict):
+                    current_object = current_object.setdefault(evaluated_part, {})
+                else:
+                    current_object = current_object[evaluated_part]
             else:
                 current_object[evaluated_part] = value
 

tests_integration/test_data/projects/snowpark_with_import/.gitignore

@@ -0,0 +1,4 @@
+.packages/
+.venv/
+app.zip
+__pycache__

tests_integration/test_data/projects/snowpark_with_import/app/functions.py

@@ -0,0 +1,7 @@
+from __future__ import annotations
+
+from dummy_pkg_for_tests import shrubbery
+
+
+def hello_function(name: str) -> str:
+    return shrubbery.knights_of_nii_says()

tests_integration/test_data/projects/snowpark_with_import/requirements.txt

@@ -0,0 +1,5 @@
+snowflake-snowpark-python
+
+# Package below should not be in snowflake anaconda
+# To assure we have full control over it we use out own dummy pkg
+# https://pypi.org/project/dummy-pkg-for-tests/

tests_integration/test_data/projects/snowpark_with_import/snowflake.yml

@@ -0,0 +1,14 @@
+definition_version: 1
+snowpark:
+  project_name: "my_snowpark_project"
+  stage_name: "TEST"
+  src: "app/"
+  functions:
+    - name: test_func
+      handler: "functions.hello_function"
+      signature:
+        - name: "name"
+          type: "string"
+      returns: string
+      imports:
+        - "@dev_deployment/dummy_pkg_for_tests.zip"

tests_integration/test_snowpark.py

@@ -142,7 +142,6 @@ def test_snowpark_flow(
             "HELLO_FUNCTION(VARCHAR) RETURN VARIANT"
         )
 
-        # Same file should be present
         _test_steps.assert_that_only_these_files_are_staged_in_test_db(
             *expected_files, stage_name=STAGE_NAME
         )
@@ -170,6 +169,48 @@ def test_snowpark_flow(
             expected_value='"Hello foo!"',
         )
 
+        # Check if adding import triggers replace
+        _test_steps.package_should_build_proper_artifact(
+            "dummy_pkg_for_tests", "dummy_pkg_for_tests/shrubbery.py"
+        )
+        _test_steps.package_should_upload_artifact_to_stage(
+            "dummy_pkg_for_tests.zip", STAGE_NAME
+        )
+
+        alter_snowflake_yml(
+            tmp_dir / "snowflake.yml",
+            parameter_path="snowpark.functions.0.imports",
+            value=["@dev_deployment/dummy_pkg_for_tests.zip"],
+        )
+
+        _test_steps.snowpark_deploy_should_finish_successfully_and_return(
+            additional_arguments=["--replace"],
+            expected_result=[
+                {
+                    "object": f"{database}.PUBLIC.HELLO_PROCEDURE(name string)",
+                    "status": "packages updated",
+                    "type": "procedure",
+                },
+                {
+                    "object": f"{database}.PUBLIC.TEST()",
+                    "status": "packages updated",
+                    "type": "procedure",
+                },
+                {
+                    "object": f"{database}.PUBLIC.HELLO_FUNCTION(name string)",
+                    "status": "definition updated",
+                    "type": "function",
+                },
+            ],
+        )
+
+        # Same file should be present, with addition of uploaded package
+        expected_files.append(f"{STAGE_NAME}/dummy_pkg_for_tests.zip")
+
+        _test_steps.assert_that_only_these_files_are_staged_in_test_db(
+            *expected_files, stage_name=STAGE_NAME
+        )
+
         # Check if objects can be dropped
         _test_steps.object_drop_should_finish_successfully(
             object_type="procedure", identifier="hello_procedure(varchar)"
@@ -205,8 +246,9 @@ def test_snowpark_with_separately_created_package(
         "dummy_pkg_for_tests.zip"
     )
 
-    with project_directory("snowpark_with_single_requirements_having_no_other_deps"):
-        _test_steps.snowpark_build_should_zip_files()
+    with project_directory("snowpark_with_import") as p_dir:
+
+        _test_steps.snowpark_build_should_zip_files(additional_files=[Path("app.zip")])
 
         _test_steps.snowpark_deploy_should_finish_successfully_and_return(
             [

tests_integration/testing_utils/snowpark_utils.py

@@ -5,7 +5,7 @@
 import re
 from enum import Enum
 from pathlib import Path
-from typing import Any, Dict, List, Tuple
+from typing import Any, Dict, List, Tuple, Optional
 from zipfile import ZipFile
 
 from syrupy import SnapshotAssertion
@@ -147,7 +147,10 @@ def object_describe_should_return_entity_description(
         )
         assert result.json is not None
 
-    def snowpark_build_should_zip_files(self, *args) -> None:
+    def snowpark_build_should_zip_files(self, *args, additional_files=None) -> None:
+        if not additional_files:
+            additional_files = [Path("requirements.other.txt")]
+
         current_files = set(Path(".").glob("**/*"))
         result = self._setup.runner.invoke_json(
             ["snowpark", "build", "--pypi-download", "yes", "--format", "JSON", *args]
@@ -161,8 +164,8 @@ def snowpark_build_should_zip_files(self, *args) -> None:
         assert_that_current_working_directory_contains_only_following_files(
             *current_files,
             Path("app.zip"),
+            *additional_files,
             Path("requirements.snowflake.txt"),
-            Path("requirements.other.txt"),
             excluded_paths=[".packages"],
         )
 
@@ -181,7 +184,7 @@ def snowpark_deploy_should_finish_successfully_and_return(
     def _run_deploy(
         self,
         expected_result: List[Dict[str, str]],
-        additional_arguments: List[str] = [],
+        additional_arguments: Optional[List[str]] = None,
     ):
         arguments = [
             "snowpark",