Skip to content

Commit

Permalink
Don't output default native_app fields in v1 to v2 migration (#1632)
Browse files Browse the repository at this point in the history
  • Loading branch information
sfc-gh-bdufour authored Sep 27, 2024
1 parent 20cec62 commit 507e5f3
Show file tree
Hide file tree
Showing 14 changed files with 235 additions and 49 deletions.
4 changes: 3 additions & 1 deletion src/snowflake/cli/_plugins/helpers/commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,9 @@ def v1_to_v2(
if pd.meets_version_requirement("2"):
return MessageResult("Project definition is already at version 2.")

pd_v2 = convert_project_definition_to_v2(manager.project_root, pd, accept_templates)
pd_v2 = convert_project_definition_to_v2(
manager.project_root, pd, accept_templates, manager.template_context
)

SecurePath("snowflake.yml").rename("snowflake_V1.yml")
with open("snowflake.yml", "w") as file:
Expand Down
60 changes: 51 additions & 9 deletions src/snowflake/cli/api/project/definition_conversion.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,15 +41,41 @@
log = logging.getLogger(__name__)


def _is_field_defined(template_context: Optional[Dict[str, Any]], *path: str) -> bool:
"""
Determines if a field is defined in the provided template context. For example,
_is_field_defined({"ctx": {"native_app": {"bundle_root": "my_root"}}}, "ctx", "native_app", "bundle_root")
returns True. If the provided template context is None, this function returns True for all paths.
"""
if template_context is None:
return True # No context, so assume that all variables are defined

current_dict = template_context
for key in path:
if not isinstance(current_dict, dict):
return False
if key not in current_dict:
return False
current_dict = current_dict[key]

return True


def convert_project_definition_to_v2(
project_root: Path, pd: ProjectDefinition, accept_templates: bool = False
project_root: Path,
pd: ProjectDefinition,
accept_templates: bool = False,
template_context: Optional[Dict[str, Any]] = None,
) -> ProjectDefinitionV2:
_check_if_project_definition_meets_requirements(pd, accept_templates)

snowpark_data = convert_snowpark_to_v2_data(pd.snowpark) if pd.snowpark else {}
streamlit_data = convert_streamlit_to_v2_data(pd.streamlit) if pd.streamlit else {}
native_app_data = (
convert_native_app_to_v2_data(project_root, pd.native_app)
convert_native_app_to_v2_data(project_root, pd.native_app, template_context)
if pd.native_app
else {}
)
Expand Down Expand Up @@ -170,7 +196,9 @@ def convert_streamlit_to_v2_data(streamlit: Streamlit) -> Dict[str, Any]:


def convert_native_app_to_v2_data(
project_root, native_app: NativeApp
project_root,
native_app: NativeApp,
template_context: Optional[Dict[str, Any]] = None,
) -> Dict[str, Any]:
def _make_meta(obj: Application | Package):
meta = {}
Expand Down Expand Up @@ -250,14 +278,24 @@ def _convert_package_script_files(package_scripts: list[str]):
"identifier": package_identifier,
"manifest": _find_manifest(),
"artifacts": native_app.artifacts,
"bundle_root": native_app.bundle_root,
"generated_root": native_app.generated_root,
"deploy_root": native_app.deploy_root,
"stage": native_app.source_stage,
"scratch_stage": native_app.scratch_stage,
}

if _is_field_defined(template_context, "ctx", "native_app", "bundle_root"):
package["bundle_root"] = native_app.bundle_root
if _is_field_defined(template_context, "ctx", "native_app", "generated_root"):
package["generated_root"] = native_app.generated_root
if _is_field_defined(template_context, "ctx", "native_app", "deploy_root"):
package["deploy_root"] = native_app.deploy_root
if _is_field_defined(template_context, "ctx", "native_app", "source_stage"):
package["stage"] = native_app.source_stage
if _is_field_defined(template_context, "ctx", "native_app", "scratch_stage"):
package["scratch_stage"] = native_app.scratch_stage

if native_app.package:
package["distribution"] = native_app.package.distribution
if _is_field_defined(
template_context, "ctx", "native_app", "package", "distribution"
):
package["distribution"] = native_app.package.distribution
package_meta = _make_meta(native_app.package)
if native_app.package.scripts:
converted_post_deploy_hooks = _convert_package_script_files(
Expand Down Expand Up @@ -289,6 +327,10 @@ def _convert_package_script_files(package_scripts: list[str]):
if native_app.application:
if app_meta := _make_meta(native_app.application):
app["meta"] = app_meta
if _is_field_defined(
template_context, "ctx", "native_app", "application", "debug"
):
app["debug"] = native_app.application.debug

return {
"entities": {
Expand Down
107 changes: 80 additions & 27 deletions tests/helpers/__snapshots__/test_v1_to_v2.ambr
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,86 @@

'''
# ---
# name: test_migrations_with_all_app_entities
'''
definition_version: '2'
entities:
pkg:
meta:
warehouse: pkg_wh
role: pkg_role
post_deploy:
- sql_script: scripts/post_pkg_deploy.sql
identifier: my_app_package
type: application package
artifacts:
- src: app/*
dest: ./
- src: to_process/*
dest: ./
processors:
- name: native app setup
- name: templates
properties:
foo: bar
bundle_root: my_output/my_bundle
deploy_root: my_output/my_deploy
generated_root: __my_generated_files
stage: app_src.my_stage
scratch_stage: app_src.my_scratch
distribution: external
manifest: app/manifest.yml
app:
meta:
warehouse: app_wh
role: app_role
post_deploy:
- sql_script: scripts/post_app_deploy.sql
identifier: myapp_app
type: application
from:
target: pkg
debug: true

'''
# ---
# name: test_migrations_with_all_app_entities.1
'''
definition_version: 1
native_app:
name: myapp
source_stage: app_src.my_stage
scratch_stage: app_src.my_scratch
bundle_root: my_output/my_bundle
deploy_root: my_output/my_deploy
generated_root: __my_generated_files
artifacts:
- src: app/*
dest: ./
- src: to_process/*
dest: ./
processors:
- native app setup
- name: templates
properties:
foo: bar
package:
name: my_app_package
role: pkg_role
warehouse: pkg_wh
distribution: external
scripts:
- scripts/post_pkg_deploy.sql
application:
name: myapp_app
role: app_role
warehouse: app_wh
debug: true
post_deploy:
- sql_script: scripts/post_app_deploy.sql

'''
# ---
# name: test_migrations_with_multiple_entities
'''
definition_version: '2'
Expand Down Expand Up @@ -236,23 +316,8 @@
artifacts:
- src: app/*
dest: ./
- src: to_process/*
dest: ./
processors:
- name: native app setup
- name: templates
properties:
foo: bar
bundle_root: output/bundle/
deploy_root: output/deploy/
generated_root: __generated/
stage: app_src.stage
scratch_stage: app_src.scratch
distribution: external
manifest: app/manifest.yml
app:
meta:
warehouse: app_wh
identifier: myapp_app
type: application
from:
Expand Down Expand Up @@ -299,25 +364,13 @@
returns: string
native_app:
name: myapp
source_stage: app_src.stage
scratch_stage: app_src.scratch
artifacts:
- src: app/*
dest: ./
- src: to_process/*
dest: ./
processors:
- native app setup
- name: templates
properties:
foo: bar
package:
role: pkg_role
distribution: external
application:
name: myapp_app
warehouse: app_wh
debug: true

'''
# ---
10 changes: 10 additions & 0 deletions tests/helpers/test_v1_to_v2.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,16 @@ def test_migrations_with_multiple_entities(
assert Path("snowflake_V1.yml").read_text() == os_agnostic_snapshot


def test_migrations_with_all_app_entities(
runner, project_directory, os_agnostic_snapshot
):
with project_directory("migration_all_app_entities"):
result = runner.invoke(["helpers", "v1-to-v2"])
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_missing_manifest(runner, project_directory):
with project_directory("migration_multiple_entities") as project_dir:
(project_dir / "app" / "manifest.yml").unlink()
Expand Down
Empty file.
16 changes: 16 additions & 0 deletions tests/test_data/projects/migration_all_app_entities/app.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
from __future__ import annotations

import sys


def hello(name: str) -> str:
return f"Hello {name}!"


# For local debugging. Be aware you may need to type-convert arguments if
# you add input parameters
if __name__ == "__main__":
if len(sys.argv) > 1:
print(hello(sys.argv[1])) # type: ignore
else:
print(hello("world"))
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
# README

This directory contains an extremely simple application that is used for
integration testing SnowCLI.
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# This is a manifest.yml file, a required component of creating a native application.
# This file defines properties required by the application package, including the location of the setup script and version definitions.
# Refer to https://docs.snowflake.com/en/developer-guide/native-apps/creating-manifest for a detailed understanding of this file.

manifest_version: 1

version:
name: dev
label: "Dev Version"
comment: "Default version used for development. Override for actual deployment."

artifacts:
setup_script: setup.sql
readme: README.md

configuration:
log_level: INFO
trace_level: ALWAYS
19 changes: 19 additions & 0 deletions tests/test_data/projects/migration_all_app_entities/app/setup.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
create application role if not exists app_public;
create or alter versioned schema core;

create or replace procedure core.echo(inp varchar)
returns varchar
language sql
immutable
as
$$
begin
return inp;
end;
$$;

grant usage on procedure core.echo(varchar) to application role app_public;

create or replace view core.shared_view as select * from my_shared_content.shared_table;

grant select on view core.shared_view to application role app_public;
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
select 1;
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
select 1;
32 changes: 32 additions & 0 deletions tests/test_data/projects/migration_all_app_entities/snowflake.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
definition_version: 1
native_app:
name: myapp
source_stage: app_src.my_stage
scratch_stage: app_src.my_scratch
bundle_root: my_output/my_bundle
deploy_root: my_output/my_deploy
generated_root: __my_generated_files
artifacts:
- src: app/*
dest: ./
- src: to_process/*
dest: ./
processors:
- native app setup
- name: templates
properties:
foo: bar
package:
name: my_app_package
role: pkg_role
warehouse: pkg_wh
distribution: external
scripts:
- scripts/post_pkg_deploy.sql
application:
name: myapp_app
role: app_role
warehouse: app_wh
debug: true
post_deploy:
- sql_script: scripts/post_app_deploy.sql
Empty file.
12 changes: 0 additions & 12 deletions tests/test_data/projects/migration_multiple_entities/snowflake.yml
Original file line number Diff line number Diff line change
Expand Up @@ -29,22 +29,10 @@ snowpark:
returns: string
native_app:
name: myapp
source_stage: app_src.stage
scratch_stage: app_src.scratch
artifacts:
- src: app/*
dest: ./
- src: to_process/*
dest: ./
processors:
- native app setup
- name: templates
properties:
foo: bar
package:
role: pkg_role
distribution: external
application:
name: myapp_app
warehouse: app_wh
debug: true

0 comments on commit 507e5f3

Please sign in to comment.