From 2f54589ce681512d59684dd47ce89edc6d7c5b86 Mon Sep 17 00:00:00 2001 From: Francois Campbell Date: Mon, 16 Sep 2024 10:40:20 -0400 Subject: [PATCH] SNOW-1643030 Append username template for v1.0 PDFs (#1556) When migrating v1.0 to v2.0, we need to emulate what the `NativeAppProjectModel` does when the app or package name are not specified, which is to append the `USER` env var to the generated identifier. --- .../cli/api/project/definition_conversion.py | 48 ++++++++++++++----- .../workspace/__snapshots__/test_manager.ambr | 12 +---- tests/workspace/test_manager.py | 28 +++++++++++ 3 files changed, 65 insertions(+), 23 deletions(-) diff --git a/src/snowflake/cli/api/project/definition_conversion.py b/src/snowflake/cli/api/project/definition_conversion.py index 1d36ec34a0..af2a983607 100644 --- a/src/snowflake/cli/api/project/definition_conversion.py +++ b/src/snowflake/cli/api/project/definition_conversion.py @@ -20,9 +20,12 @@ from snowflake.cli.api.project.schemas.entities.common import ( SqlScriptHookType, ) -from snowflake.cli.api.project.schemas.native_app.application import Application +from snowflake.cli.api.project.schemas.native_app.application import ( + Application, + ApplicationV11, +) from snowflake.cli.api.project.schemas.native_app.native_app import NativeApp -from snowflake.cli.api.project.schemas.native_app.package import Package +from snowflake.cli.api.project.schemas.native_app.package import Package, PackageV11 from snowflake.cli.api.project.schemas.project_definition import ( ProjectDefinition, ProjectDefinitionV2, @@ -206,6 +209,9 @@ def _find_manifest(): # which use POSIX paths as default values return manifest_path.as_posix() + def _make_template(template: str) -> str: + return f"{PROJECT_TEMPLATE_VARIABLE_OPENING} {template} {PROJECT_TEMPLATE_VARIABLE_CLOSING}" + def _convert_package_script_files(package_scripts: list[str]): # PDFv2 doesn't support package scripts, only post-deploy scripts, so we # need to convert the Jinja syntax from {{ }} to <% %> @@ -213,7 +219,9 @@ def _convert_package_script_files(package_scripts: list[str]): # to v2 template syntax by running it though the template process with a fake # package name that's actually a valid v2 template, which will be evaluated # when the script is used as a post-deploy script - fake_package_replacement_template = f"{PROJECT_TEMPLATE_VARIABLE_OPENING} ctx.entities.{package_entity_name}.identifier {PROJECT_TEMPLATE_VARIABLE_CLOSING}" + fake_package_replacement_template = _make_template( + f"ctx.entities.{package_entity_name}.identifier" + ) jinja_context = dict(package_name=fake_package_replacement_template) post_deploy_hooks = [] for script_file in package_scripts: @@ -225,13 +233,20 @@ def _convert_package_script_files(package_scripts: list[str]): return post_deploy_hooks package_entity_name = "pkg" + if ( + native_app.package + and native_app.package.name + and native_app.package.name != PackageV11.model_fields["name"].default + ): + package_identifier = native_app.package.name + else: + # Backport the PackageV11 default name template, updated for PDFv2 + package_identifier = _make_template( + f"fn.concat_ids('{native_app.name}', '_pkg_', fn.sanitize_id(fn.get_username('unknown_user')) | lower)" + ) package = { "type": "application package", - "identifier": ( - native_app.package.name - if native_app.package and native_app.package.name - else f"{native_app.name}_pkg" - ), + "identifier": package_identifier, "manifest": _find_manifest(), "artifacts": native_app.artifacts, "bundle_root": native_app.bundle_root, @@ -254,13 +269,20 @@ def _convert_package_script_files(package_scripts: list[str]): package["meta"] = package_meta app_entity_name = "app" + if ( + native_app.application + and native_app.application.name + and native_app.application.name != ApplicationV11.model_fields["name"].default + ): + app_identifier = native_app.application.name + else: + # Backport the ApplicationV11 default name template, updated for PDFv2 + app_identifier = _make_template( + f"fn.concat_ids('{native_app.name}', '_', fn.sanitize_id(fn.get_username('unknown_user')) | lower)" + ) app = { "type": "application", - "identifier": ( - native_app.application.name - if native_app.application and native_app.application.name - else native_app.name - ), + "identifier": app_identifier, "from": {"target": package_entity_name}, } if native_app.application: diff --git a/tests/workspace/__snapshots__/test_manager.ambr b/tests/workspace/__snapshots__/test_manager.ambr index f76fe996dd..913229f454 100644 --- a/tests/workspace/__snapshots__/test_manager.ambr +++ b/tests/workspace/__snapshots__/test_manager.ambr @@ -168,15 +168,6 @@ ''' # --- -# name: test_migrating_native_app_raises_error - ''' - +- Error ----------------------------------------------------------------------+ - | Your project file contains a native app definition. Conversion of Native | - | apps is not yet supported | - +------------------------------------------------------------------------------+ - - ''' -# --- # name: test_migrations_with_multiple_entities ''' definition_version: '2' @@ -226,7 +217,8 @@ deploy_root: output/deploy/ distribution: external generated_root: __generated/ - identifier: myapp_pkg + identifier: <% fn.concat_ids('myapp', '_pkg_', fn.sanitize_id(fn.get_username('unknown_user')) + | lower) %> manifest: app/manifest.yml meta: role: pkg_role diff --git a/tests/workspace/test_manager.py b/tests/workspace/test_manager.py index 30c1e46833..a95425f30d 100644 --- a/tests/workspace/test_manager.py +++ b/tests/workspace/test_manager.py @@ -221,3 +221,31 @@ def test_migrating_a_file_with_duplicated_keys_raises_an_error( result = runner.invoke(["ws", "migrate"]) assert result.exit_code == 1 assert result.output == os_agnostic_snapshot + + +@pytest.mark.parametrize("definition_version", [1, "1.1"]) +def test_migrate_nativeapp_fields_with_username( + runner, project_directory, definition_version +): + with project_directory("integration") as pd: + definition_path = pd / "snowflake.yml" + with definition_path.open("r+") as f: + old_definition = yaml.safe_load(f) + old_definition["definition_version"] = definition_version + f.seek(0) + yaml.safe_dump(old_definition, f) + f.truncate() + + result = runner.invoke(["ws", "migrate", "--accept-templates"]) + assert result.exit_code == 0, result.output + + with definition_path.open("r") as f: + new_definition = yaml.safe_load(f) + assert ( + new_definition["entities"]["app"]["identifier"] + == "<% fn.concat_ids('integration', '_', fn.sanitize_id(fn.get_username('unknown_user')) | lower) %>" + ) + assert ( + new_definition["entities"]["pkg"]["identifier"] + == "<% fn.concat_ids('integration', '_pkg_', fn.sanitize_id(fn.get_username('unknown_user')) | lower) %>" + )