Skip to content

Commit

Permalink
SNOW-1643030 Append username template for v1.0 PDFs (#1556)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
sfc-gh-fcampbell committed Sep 16, 2024
1 parent 2d33807 commit 2f54589
Show file tree
Hide file tree
Showing 3 changed files with 65 additions and 23 deletions.
48 changes: 35 additions & 13 deletions src/snowflake/cli/api/project/definition_conversion.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -206,14 +209,19 @@ 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 <% %>
# Luckily, package scripts only support {{ package_name }}, so let's convert that tag
# 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:
Expand All @@ -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,
Expand All @@ -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:
Expand Down
12 changes: 2 additions & 10 deletions tests/workspace/__snapshots__/test_manager.ambr
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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
Expand Down
28 changes: 28 additions & 0 deletions tests/workspace/test_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -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) %>"
)

0 comments on commit 2f54589

Please sign in to comment.