From 43da8c984d816dfaadccd03cace5aac89e87cf9c Mon Sep 17 00:00:00 2001 From: PARYA JAFARI Date: Thu, 14 Nov 2024 13:36:33 -0500 Subject: [PATCH 1/4] make manifest optional, don't add manifest in v1-to-v2 conversion --- .../nativeapp/entities/application_package.py | 4 +- .../cli/api/project/definition_conversion.py | 40 +----- .../helpers/__snapshots__/test_v1_to_v2.ambr | 123 +++++++++++++++++- tests/helpers/test_v1_to_v2.py | 40 +----- tests/project/test_project_definition_v2.py | 1 - 5 files changed, 130 insertions(+), 78 deletions(-) diff --git a/src/snowflake/cli/_plugins/nativeapp/entities/application_package.py b/src/snowflake/cli/_plugins/nativeapp/entities/application_package.py index f057b08fb2..d0a586a58b 100644 --- a/src/snowflake/cli/_plugins/nativeapp/entities/application_package.py +++ b/src/snowflake/cli/_plugins/nativeapp/entities/application_package.py @@ -112,9 +112,7 @@ class ApplicationPackageEntityModel(EntityModelBase): title="Distribution of the application package created by the Snowflake CLI", default="internal", ) - manifest: str = Field( - title="Path to manifest.yml", - ) + manifest: Optional[str] = Field(title="Path to manifest.yml", default="") @field_validator("identifier") @classmethod diff --git a/src/snowflake/cli/api/project/definition_conversion.py b/src/snowflake/cli/api/project/definition_conversion.py index 28b2a74c64..1f76e998d2 100644 --- a/src/snowflake/cli/api/project/definition_conversion.py +++ b/src/snowflake/cli/api/project/definition_conversion.py @@ -10,7 +10,6 @@ from click import ClickException from snowflake.cli._plugins.nativeapp.artifacts import ( - BundleMap, bundle_artifacts, ) from snowflake.cli._plugins.nativeapp.bundle_context import BundleContext @@ -118,9 +117,7 @@ def convert_project_definition_to_v2( else {} ) native_app_data, native_app_template_context = ( - convert_native_app_to_v2_data( - project_root, definition_v1.native_app, template_context - ) + convert_native_app_to_v2_data(definition_v1.native_app, template_context) if definition_v1.native_app else ({}, {}) ) @@ -262,7 +259,6 @@ def convert_streamlit_to_v2_data(streamlit: Streamlit) -> Dict[str, Any]: def convert_native_app_to_v2_data( - project_root: Path, native_app: NativeApp, template_context: Optional[Dict[str, Any]] = None, ) -> tuple[dict[str, Any], dict[str, Any]]: @@ -276,39 +272,6 @@ def _make_meta(obj: Application | Package): meta["post_deploy"] = obj.post_deploy return meta - def _find_manifest(): - # We don't know which file in the project directory is the actual manifest, - # and we can't iterate through the artifacts property since the src can contain - # glob patterns. The simplest solution is to bundle the app and find the - # manifest file from the resultant BundleMap, since the bundle process ensures - # that only a single source path can map to the corresponding destination path - bundle_map = BundleMap( - project_root=project_root, deploy_root=project_root / native_app.deploy_root - ) - for artifact in native_app.artifacts: - bundle_map.add(artifact) - - manifest_path = next( - ( - src - for src, dest in bundle_map.all_mappings( - absolute=True, expand_directories=True - ) - if dest.name == "manifest.yml" - ), - None, - ) - if not manifest_path: - # The manifest field is required, so we can't gracefully handle it being missing - raise ClickException( - "manifest.yml file not found in any Native App artifact sources, " - "unable to perform migration" - ) - - # Use a POSIX path to be consistent with other migrated fields - # which use POSIX paths as default values - return manifest_path.relative_to(project_root).as_posix() - package_entity_name = "pkg" if ( native_app.package @@ -324,7 +287,6 @@ def _find_manifest(): package = { "type": "application package", "identifier": package_identifier, - "manifest": _find_manifest(), "artifacts": native_app.artifacts, } diff --git a/tests/helpers/__snapshots__/test_v1_to_v2.ambr b/tests/helpers/__snapshots__/test_v1_to_v2.ambr index 780509d615..2e7458624b 100644 --- a/tests/helpers/__snapshots__/test_v1_to_v2.ambr +++ b/tests/helpers/__snapshots__/test_v1_to_v2.ambr @@ -168,6 +168,127 @@ ''' # --- +# name: test_migration_native_app_no_artifacts + ''' + definition_version: '2' + entities: + procedureName: + imports: [] + external_access_integrations: [] + secrets: {} + meta: + use_mixins: + - snowpark_shared + identifier: + name: procedureName + handler: hello + returns: string + signature: + - name: name + type: string + stage: dev_deployment + artifacts: + - src: app + dest: my_snowpark_project + type: procedure + execute_as_caller: false + func1: + imports: [] + external_access_integrations: [] + secrets: {} + meta: + use_mixins: + - snowpark_shared + identifier: + name: func1 + handler: app.func1_handler + returns: string + signature: + - name: a + type: string + default: default value + - name: b + type: variant + runtime: '3.1' + stage: dev_deployment + artifacts: + - src: app + dest: my_snowpark_project + type: function + test_streamlit: + identifier: + name: test_streamlit + type: streamlit + title: My Fancy Streamlit + query_warehouse: test_warehouse + main_file: streamlit_app.py + pages_dir: None + stage: streamlit + artifacts: + - streamlit_app.py + - environment.yml + - pages + pkg: + meta: + role: pkg_role + identifier: <% fn.concat_ids('myapp', '_pkg_', fn.sanitize_id(fn.get_username('unknown_user')) | lower) %> + type: application package + artifacts: [] + app: + identifier: myapp_app + type: application + from: + target: pkg + mixins: + snowpark_shared: + stage: dev_deployment + artifacts: + - src: app/ + dest: my_snowpark_project + + ''' +# --- +# name: test_migration_native_app_no_artifacts.1 + ''' + definition_version: 1 + native_app: + application: + name: myapp_app + artifacts: [] + name: myapp + package: + role: pkg_role + snowpark: + functions: + - handler: app.func1_handler + name: func1 + returns: string + runtime: 3.1 + signature: + - default: default value + name: a + type: string + - name: b + type: variant + procedures: + - handler: hello + name: procedureName + returns: string + signature: + - name: name + type: string + project_name: my_snowpark_project + src: app/ + stage_name: dev_deployment + streamlit: + main_file: streamlit_app.py + name: test_streamlit + query_warehouse: test_warehouse + stage: streamlit + title: My Fancy Streamlit + + ''' +# --- # name: test_migrations_with_all_app_entities ''' definition_version: '2' @@ -196,7 +317,6 @@ stage: app_src.my_stage scratch_stage: app_src.my_scratch distribution: external - manifest: app/manifest.yml app: meta: warehouse: app_wh @@ -316,7 +436,6 @@ artifacts: - src: app/* dest: ./ - manifest: app/manifest.yml app: identifier: myapp_app type: application diff --git a/tests/helpers/test_v1_to_v2.py b/tests/helpers/test_v1_to_v2.py index 01b0069ea7..76a3e86d8f 100644 --- a/tests/helpers/test_v1_to_v2.py +++ b/tests/helpers/test_v1_to_v2.py @@ -1,5 +1,4 @@ import logging -import shutil from pathlib import Path from textwrap import dedent @@ -38,36 +37,10 @@ def test_migrations_with_all_app_entities( assert Path("snowflake_V1.yml").read_text() == os_agnostic_snapshot -def test_migration_native_app_nested_manifest(runner, project_directory): - with project_directory("migration_multiple_entities") as project_dir: - with open("snowflake.yml", "r+") as snowflake_yml: - pdf = yaml.safe_load(snowflake_yml) - pdf["native_app"]["artifacts"] = [ - dict(src="nested/nested/app", dest="nested/app/") - ] - snowflake_yml.seek(0) - yaml.safe_dump(pdf, snowflake_yml) - snowflake_yml.truncate() - - shutil.move("app", "nested/nested/app") - result = runner.invoke(["helpers", "v1-to-v2"]) - assert result.exit_code == 0 - with open("snowflake.yml") as f: - pdf = yaml.safe_load(f) - assert ( - pdf["entities"]["pkg"]["manifest"] == "nested/nested/app/manifest.yml" - ) - - -def test_migration_native_app_missing_manifest(runner, project_directory): - with project_directory("migration_multiple_entities") as project_dir: - (project_dir / "app" / "manifest.yml").unlink() - result = runner.invoke(["helpers", "v1-to-v2"]) - assert result.exit_code == 1 - assert "manifest.yml file not found" in result.output - - -def test_migration_native_app_no_artifacts(runner, project_directory): +# Migration of app without artifacts shouldn't fail +def test_migration_native_app_no_artifacts( + runner, project_directory, os_agnostic_snapshot +): with project_directory("migration_multiple_entities") as project_dir: with (project_dir / "snowflake.yml").open("r+") as snowflake_yml: pdf = yaml.safe_load(snowflake_yml) @@ -76,8 +49,9 @@ def test_migration_native_app_no_artifacts(runner, project_directory): yaml.safe_dump(pdf, snowflake_yml) snowflake_yml.truncate() result = runner.invoke(["helpers", "v1-to-v2"]) - assert result.exit_code == 1 - assert "manifest.yml file not found" in result.output + assert result.exit_code == 0 + assert Path("snowflake.yml").read_text() == os_agnostic_snapshot + assert Path("snowflake_V1.yml").read_text() == os_agnostic_snapshot def test_migration_native_app_package_scripts(runner, project_directory): diff --git a/tests/project/test_project_definition_v2.py b/tests/project/test_project_definition_v2.py index 9366913ce3..f54497ae74 100644 --- a/tests/project/test_project_definition_v2.py +++ b/tests/project/test_project_definition_v2.py @@ -57,7 +57,6 @@ {"entities": {"pkg": {"type": "application package"}}}, [ "missing the following field: 'entities.pkg.application package.artifacts'", - "missing the following field: 'entities.pkg.application package.manifest'", ], ], [ From f9798b76a7fdcc24180f45d5bd0ebaab91cf6a9e Mon Sep 17 00:00:00 2001 From: PARYA JAFARI Date: Thu, 14 Nov 2024 13:48:39 -0500 Subject: [PATCH 2/4] update release notes --- RELEASE-NOTES.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/RELEASE-NOTES.md b/RELEASE-NOTES.md index 6d2dad8c64..ac1f3b5c28 100644 --- a/RELEASE-NOTES.md +++ b/RELEASE-NOTES.md @@ -17,7 +17,7 @@ ## Backward incompatibility ## Deprecations - +* `manifest` field of `application package` entity is now optional. This field does not have any functionality. ## New additions * Added `--retain-comments` option to `snow sql` command to allow passing comments to Snowflake. * Added `--replace` and `--if-not-exists` options to `snow object create` command. From a9d95a4ead339321e850dbb575bdd17a08b224a9 Mon Sep 17 00:00:00 2001 From: PARYA JAFARI Date: Fri, 15 Nov 2024 10:44:46 -0500 Subject: [PATCH 3/4] change title --- .../cli/_plugins/nativeapp/entities/application_package.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/snowflake/cli/_plugins/nativeapp/entities/application_package.py b/src/snowflake/cli/_plugins/nativeapp/entities/application_package.py index d0a586a58b..029efeaeac 100644 --- a/src/snowflake/cli/_plugins/nativeapp/entities/application_package.py +++ b/src/snowflake/cli/_plugins/nativeapp/entities/application_package.py @@ -112,7 +112,9 @@ class ApplicationPackageEntityModel(EntityModelBase): title="Distribution of the application package created by the Snowflake CLI", default="internal", ) - manifest: Optional[str] = Field(title="Path to manifest.yml", default="") + manifest: Optional[str] = Field( + title="Unused field. Path to manifest.yml", default="" + ) @field_validator("identifier") @classmethod From 2390f6624e7c7c0ef1af6fa9afa3e263f171dffe Mon Sep 17 00:00:00 2001 From: PARYA JAFARI Date: Fri, 15 Nov 2024 11:41:35 -0500 Subject: [PATCH 4/4] reword title for manifest --- .../cli/_plugins/nativeapp/entities/application_package.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/snowflake/cli/_plugins/nativeapp/entities/application_package.py b/src/snowflake/cli/_plugins/nativeapp/entities/application_package.py index 029efeaeac..68bd843f7e 100644 --- a/src/snowflake/cli/_plugins/nativeapp/entities/application_package.py +++ b/src/snowflake/cli/_plugins/nativeapp/entities/application_package.py @@ -113,7 +113,8 @@ class ApplicationPackageEntityModel(EntityModelBase): default="internal", ) manifest: Optional[str] = Field( - title="Unused field. Path to manifest.yml", default="" + title="Path to manifest.yml. Unused and deprecated starting with Snowflake CLI 3.2", + default="", ) @field_validator("identifier")