Skip to content

Commit

Permalink
Basic solution
Browse files Browse the repository at this point in the history
SNOW-1636849 Auto-teardown Native App in integration tests (#1478)

Changes `with project_directory()` to `with nativeapp_project_directory()`, which automatically runs `snow app teardown` before exiting the project. This allows us to remove the `try`/`finally` in most tests. For tests that were using `with pushd(test_project)`, this has been changed to `with nativeapp_teardown()`, which is what `with nativeapp_project_directory()` uses under the hood.

SNOW-1621834 Cast version to identifier when creating/dropping app versions (#1475)

When running `snow app version create` and `snow app version drop`, wrap the version in `to_identifier()` so users don't have to specify the quotes around version names that aren't valid identifiers. If the name is already quoted, `to_identifier()` doesn't do anything.

Added tests

Added tests
  • Loading branch information
sfc-gh-jsikorski committed Aug 25, 2024
1 parent 5e929c0 commit 73fa219
Show file tree
Hide file tree
Showing 25 changed files with 766 additions and 798 deletions.
1 change: 1 addition & 0 deletions RELEASE-NOTES.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
* Fix the typo in spcs service name argument description. It is the identifier of the **service** instead of the **service pool**.
* Fix error handling and improve messaging when no artifacts provided.
* Improved error message for incompatible parameters.
* Fixed SQL error when running `snow app version create` and `snow app version drop` with a version name that isn't a valid Snowflake unquoted identifier


# v2.8.0
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,9 @@
from snowflake.cli.api.console import cli_console as cc
from snowflake.cli.api.exceptions import SnowflakeSQLExecutionError
from snowflake.cli.api.project.schemas.native_app.native_app import NativeApp
from snowflake.cli.api.project.util import unquote_identifier
from snowflake.cli.api.project.util import to_identifier, unquote_identifier
from snowflake.cli.api.utils.cursor import (
find_all_rows,
find_first_row,
)
from snowflake.connector import ProgrammingError
from snowflake.connector.cursor import DictCursor
Expand Down Expand Up @@ -121,6 +120,8 @@ def add_new_version(self, version: str) -> None:
"""
Defines a new version in an existing application package.
"""
# Make the version a valid identifier, adding quotes if necessary
version = to_identifier(version)
with self.use_role(self.package_role):
cc.step(
f"Defining a new version {version} in application package {self.package_name}"
Expand All @@ -141,6 +142,8 @@ def add_new_patch_to_version(self, version: str, patch: Optional[int] = None):
"""
Add a new patch, optionally a custom one, to an existing version in an application package.
"""
# Make the version a valid identifier, adding quotes if necessary
version = to_identifier(version)
with self.use_role(self.package_role):
cc.step(
f"Adding new patch to version {version} defined in application package {self.package_name}"
Expand All @@ -156,11 +159,7 @@ def add_new_patch_to_version(self, version: str, patch: Optional[int] = None):
add_version_query, cursor_class=DictCursor
)

show_row = find_first_row(
result_cursor,
lambda row: row[VERSION_COL] == unquote_identifier(version),
)

show_row = result_cursor.fetchall()[0]
new_patch = show_row["patch"]
cc.message(
f"Patch {new_patch} created for version {version} defined in application package {self.package_name}."
Expand Down Expand Up @@ -328,6 +327,9 @@ def process(
"Manifest.yml file does not contain a value for the version field."
)

# Make the version a valid identifier, adding quotes if necessary
version = to_identifier(version)

cc.step(
dedent(
f"""\
Expand Down
4 changes: 4 additions & 0 deletions src/snowflake/cli/api/project/schemas/entities/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,10 @@ class MetaField(UpdatableModel):
title="Actions that will be executed after the application object is created/upgraded",
default=None,
)
use_mixin: Optional[str] = Field(
title="Name of the mixin used to fill the entity fields",
default=None,
)


class DefaultsField(UpdatableModel):
Expand Down
20 changes: 20 additions & 0 deletions src/snowflake/cli/api/project/schemas/project_definition.py
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,26 @@ def _validate_target_field(
default=None,
)

mixins: Optional[Dict[str, Dict]] = Field(
title="Mixins to apply to entities",
default=None,
)

@model_validator(mode="before")
@classmethod
def apply_mixins(cls, data: Dict) -> Dict:
"""
Applies mixins to those entities, whose meta field contains the mixin name.
"""
if "mixins" in data and "entities" in data:
for name, mixin in data.get("mixins", {}).items():
for entity in data["entities"].values():
if entity.get("meta", {}).get("use_mixin") == name:
for key in mixin.keys():
if key in entity.keys():
entity[key] = mixin[key]
return data

def get_entities_by_type(self, entity_type: str):
return {i: e for i, e in self.entities.items() if e.get_type() == entity_type}

Expand Down
37 changes: 25 additions & 12 deletions tests/nativeapp/test_version_create_processor.py
Original file line number Diff line number Diff line change
Expand Up @@ -105,8 +105,11 @@ def test_get_existing_release_direction_info(mock_execute, temp_dir, mock_cursor

# Test add_new_version adds a new version to an app pkg correctly
@mock.patch(NATIVEAPP_MANAGER_EXECUTE)
def test_add_version(mock_execute, temp_dir, mock_cursor):
version = "V1"
@pytest.mark.parametrize(
["version", "version_identifier"],
[("V1", "V1"), ("1.0.0", '"1.0.0"'), ('"1.0.0"', '"1.0.0"')],
)
def test_add_version(mock_execute, temp_dir, mock_cursor, version, version_identifier):
side_effects, expected = mock_execute_helper(
[
(
Expand All @@ -120,7 +123,7 @@ def test_add_version(mock_execute, temp_dir, mock_cursor):
dedent(
f"""\
alter application package app_pkg
add version V1
add version {version_identifier}
using @app_pkg.app_src.stage
"""
),
Expand All @@ -146,8 +149,13 @@ def test_add_version(mock_execute, temp_dir, mock_cursor):

# Test add_new_patch_to_version adds an "auto-increment" patch to an existing version
@mock.patch(NATIVEAPP_MANAGER_EXECUTE)
def test_add_new_patch_auto(mock_execute, temp_dir, mock_cursor):
version = "V1"
@pytest.mark.parametrize(
["version", "version_identifier"],
[("V1", "V1"), ("1.0.0", '"1.0.0"'), ('"1.0.0"', '"1.0.0"')],
)
def test_add_new_patch_auto(
mock_execute, temp_dir, mock_cursor, version, version_identifier
):
side_effects, expected = mock_execute_helper(
[
(
Expand All @@ -161,7 +169,7 @@ def test_add_new_patch_auto(mock_execute, temp_dir, mock_cursor):
dedent(
f"""\
alter application package app_pkg
add patch for version V1
add patch for version {version_identifier}
using @app_pkg.app_src.stage
"""
),
Expand All @@ -187,8 +195,13 @@ def test_add_new_patch_auto(mock_execute, temp_dir, mock_cursor):

# Test add_new_patch_to_version adds a custom patch to an existing version
@mock.patch(NATIVEAPP_MANAGER_EXECUTE)
def test_add_new_patch_custom(mock_execute, temp_dir, mock_cursor):
version = "V1"
@pytest.mark.parametrize(
["version", "version_identifier"],
[("V1", "V1"), ("1.0.0", '"1.0.0"'), ('"1.0.0"', '"1.0.0"')],
)
def test_add_new_patch_custom(
mock_execute, temp_dir, mock_cursor, version, version_identifier
):
side_effects, expected = mock_execute_helper(
[
(
Expand All @@ -202,7 +215,7 @@ def test_add_new_patch_custom(mock_execute, temp_dir, mock_cursor):
dedent(
f"""\
alter application package app_pkg
add patch 12 for version V1
add patch 12 for version {version_identifier}
using @app_pkg.app_src.stage
"""
),
Expand All @@ -222,7 +235,7 @@ def test_add_new_patch_custom(mock_execute, temp_dir, mock_cursor):
)

processor = _get_version_create_processor()
processor.add_new_patch_to_version(version, "12")
processor.add_new_patch_to_version(version, 12)
assert mock_execute.mock_calls == expected


Expand Down Expand Up @@ -282,7 +295,7 @@ def test_process_no_version_exists_throws_bad_option_exception_one(
processor.process(
bundle_map=mock_bundle_map,
version="v1",
patch="12",
patch=12,
policy=policy_param,
git_policy=policy_param,
is_interactive=False,
Expand Down Expand Up @@ -315,7 +328,7 @@ def test_process_no_version_exists_throws_bad_option_exception_two(
processor.process(
bundle_map=mock_bundle_map,
version="v1",
patch="12",
patch=12,
policy=policy_param,
git_policy=policy_param,
is_interactive=False,
Expand Down
83 changes: 76 additions & 7 deletions tests/nativeapp/test_version_drop_processor.py
Original file line number Diff line number Diff line change
Expand Up @@ -96,13 +96,13 @@ def test_process_has_no_existing_app_pkg(mock_get_existing, policy_param, temp_d
def test_process_no_version_from_user_no_version_in_manifest(
mock_version_info_in_manifest,
mock_build_bundle,
mock_mismatch,
mock_distribution,
mock_get_existing,
policy_param,
temp_dir,
):

mock_mismatch.return_Value = "internal"
mock_distribution.return_value = "internal"
current_working_directory = os.getcwd()
create_named_file(
file_name="snowflake.yml",
Expand Down Expand Up @@ -144,15 +144,15 @@ def test_process_drop_cannot_complete(
mock_typer_confirm,
mock_version_info_in_manifest,
mock_build_bundle,
mock_mismatch,
mock_distribution,
mock_get_existing,
policy_param,
is_interactive_param,
expected_code,
temp_dir,
):

mock_mismatch.return_value = "internal"
mock_distribution.return_value = "internal"
current_working_directory = os.getcwd()
create_named_file(
file_name="snowflake.yml",
Expand Down Expand Up @@ -190,20 +190,20 @@ def test_process_drop_cannot_complete(
(ask_always_policy, True),
],
)
def test_process_drop_success(
def test_process_drop_from_manifest(
mock_typer_confirm,
mock_execute,
mock_version_info_in_manifest,
mock_build_bundle,
mock_mismatch,
mock_distribution,
mock_get_existing,
policy_param,
is_interactive_param,
temp_dir,
mock_cursor,
):

mock_mismatch.return_value = "internal"
mock_distribution.return_value = "internal"
side_effects, expected = mock_execute_helper(
[
(
Expand Down Expand Up @@ -234,3 +234,72 @@ def test_process_drop_success(
version=None, policy=policy_param, is_interactive=is_interactive_param
)
assert mock_execute.mock_calls == expected


@mock.patch(
f"{VERSION_MODULE}.{DROP_PROCESSOR}.get_existing_app_pkg_info",
return_value={"owner": "package_role"},
)
@mock_get_app_pkg_distribution_in_sf()
@mock.patch(f"{VERSION_MODULE}.{DROP_PROCESSOR}.build_bundle", return_value=None)
@mock.patch(NATIVEAPP_MANAGER_EXECUTE)
@mock.patch(
f"snowflake.cli._plugins.nativeapp.policy.{TYPER_CONFIRM}", return_value=True
)
@pytest.mark.parametrize(
"policy_param, is_interactive_param",
[
(allow_always_policy, False),
(ask_always_policy, True),
(ask_always_policy, True),
],
)
@pytest.mark.parametrize(
["version", "version_identifier"],
[("V1", "V1"), ("1.0.0", '"1.0.0"'), ('"1.0.0"', '"1.0.0"')],
)
def test_process_drop_specific_version(
mock_typer_confirm,
mock_execute,
mock_build_bundle,
mock_distribution,
mock_get_existing,
policy_param,
is_interactive_param,
temp_dir,
mock_cursor,
version,
version_identifier,
):

mock_distribution.return_value = "internal"
side_effects, expected = mock_execute_helper(
[
(
mock_cursor([{"CURRENT_ROLE()": "old_role"}], []),
mock.call("select current_role()", cursor_class=DictCursor),
),
(None, mock.call("use role package_role")),
(
None,
mock.call(
f"alter application package app_pkg drop version {version_identifier}"
),
),
(None, mock.call("use role old_role")),
]
)
mock_execute.side_effect = side_effects

current_working_directory = os.getcwd()
create_named_file(
file_name="snowflake.yml",
dir_name=current_working_directory,
contents=[mock_snowflake_yml_file],
)

processor = _get_version_drop_processor()
processor.process(
version=version, policy=policy_param, is_interactive=is_interactive_param
)
assert mock_execute.mock_calls == expected
26 changes: 26 additions & 0 deletions tests/project/test_project_definition_v2.py
Original file line number Diff line number Diff line change
Expand Up @@ -385,6 +385,32 @@ def test_v1_to_v2_conversion(
_assert_entities_are_equal(v1_function, v2_function)


@pytest.mark.parametrize(
"project_name,stage1,stage2",
[("mixins_basic", "foo", "bar"), ("mixins_defaults_hierarchy", "foo", "baz")],
)
def test_mixins(project_directory, project_name, stage1, stage2):
with project_directory(project_name) as project_dir:
definition = DefinitionManager(project_dir).project_definition

assert definition.entities["function1"].stage == stage1
assert definition.entities["function2"].stage == stage2


def test_mixins_for_different_entities(project_directory):
with project_directory("mixins_different_entities") as project_dir:
definition = DefinitionManager(project_dir).project_definition

assert definition.entities["function1"].stage == "foo"
assert definition.entities["streamlit1"].main_file == "streamlit_app.py"


# TODO: add tests for the following:
# - entities of different types - only entity specific fields should be updated
# - entities with different mixins
# - which prevailes - defaults or mixins? Check order


def _assert_entities_are_equal(
v1_entity: _CallableBase, v2_entity: SnowparkEntityModel
):
Expand Down
28 changes: 28 additions & 0 deletions tests/test_data/projects/mixins_basic/snowflake.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
definition_version: '2'
entities:
function1:
artifacts:
- src
handler: app.hello
identifier: name
meta:
use_mixin: my_mixin
returns: string
signature:
- name: name
type: string
type: function
function2:
artifacts:
- src
handler: app.hello
identifier: name
returns: string
signature:
- name: name
type: string
stage: bar
type: function
mixins:
my_mixin:
stage: foo
Loading

0 comments on commit 73fa219

Please sign in to comment.