Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix numerical version when reading from manifest file #1602

Merged
merged 5 commits into from
Sep 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 9 additions & 8 deletions src/snowflake/cli/_plugins/nativeapp/artifacts.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
from click.exceptions import ClickException
from snowflake.cli.api.constants import DEFAULT_SIZE_LIMIT_MB
from snowflake.cli.api.project.schemas.v1.native_app.path_mapping import PathMapping
from snowflake.cli.api.project.util import to_identifier
from snowflake.cli.api.secure_path import SecurePath
from yaml import safe_load

Expand Down Expand Up @@ -734,23 +735,23 @@ def find_setup_script_file(deploy_root: Path) -> Path:

def find_version_info_in_manifest_file(
deploy_root: Path,
) -> Tuple[Optional[str], Optional[str]]:
) -> Tuple[Optional[str], Optional[int]]:
"""
Find version and patch, if available, in the manifest.yml file.
"""
version_field = "version"
name_field = "name"
patch_field = "patch"

manifest_content = find_and_read_manifest_file(deploy_root=deploy_root)

version_name: Optional[str] = None
patch_name: Optional[str] = None
patch_number: Optional[int] = None

if version_field in manifest_content:
version_info = manifest_content[version_field]
version_name = version_info[name_field]
version_info = manifest_content.get("version", None)
if version_info:
if name_field in version_info:
version_name = to_identifier(str(version_info[name_field]))
if patch_field in version_info:
patch_name = version_info[patch_field]
patch_number = int(version_info[patch_field])

return version_name, patch_name
return version_name, patch_number
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ def process(
)

# Check if --patch needs to throw a bad option error, either if application package does not exist or if version does not exist
if patch:
if patch is not None:
try:
if not self.get_existing_version_info(version):
raise BadOptionUsage(
Expand Down
32 changes: 32 additions & 0 deletions tests/nativeapp/test_artifacts.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,14 @@
SourceNotFoundError,
TooManyFilesError,
build_bundle,
find_version_info_in_manifest_file,
resolve_without_follow,
symlink_or_copy,
)
from snowflake.cli.api.project.definition import load_project
from snowflake.cli.api.project.schemas.v1.native_app.path_mapping import PathMapping
from snowflake.cli.api.project.util import to_identifier
from yaml import safe_dump

from tests.nativeapp.utils import (
assert_dir_snapshot,
Expand Down Expand Up @@ -1360,3 +1363,32 @@ def test_symlink_or_copy_with_symlinks_in_project_root(os_agnostic_snapshot):
)

assert_dir_snapshot(Path("./output/deploy"), os_agnostic_snapshot)


@pytest.mark.parametrize("patch_name", [None, "1", 42])
@pytest.mark.parametrize(
"version_name", [None, "v1", "1", 2, "1.2", 1.3, "1.2.3", "0.x", "foo", "abc def"]
)
def test_find_version_info_in_manifest_file(version_name, patch_name):
manifest_contents = {"manifest_version": 1, "version": {}}

if version_name is None and patch_name is None:
manifest_contents.pop("version")
if version_name is not None:
manifest_contents["version"]["name"] = version_name
if patch_name is not None:
manifest_contents["version"]["patch"] = patch_name

deploy_root_structure = {"manifest.yml": safe_dump(manifest_contents)}
with temp_local_dir(deploy_root_structure) as deploy_root:
v, p = find_version_info_in_manifest_file(deploy_root=deploy_root)

if version_name is None:
assert v is None
else:
assert v == to_identifier(str(version_name))

if patch_name is None:
assert p is None
else:
assert p == int(patch_name)
106 changes: 106 additions & 0 deletions tests_integration/nativeapp/test_version.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,42 @@
# See the License for the specific language governing permissions and
# limitations under the License.
from shlex import split
from typing import Any, Optional, Union

from yaml import safe_dump, safe_load

from snowflake.cli.api.project.util import (
is_valid_unquoted_identifier,
to_identifier,
unquote_identifier,
)
from tests.project.fixtures import *
from tests_integration.test_utils import contains_row_with, row_from_snowflake_session


def set_version_in_app_manifest(manifest_path: Path, version: Any, patch: Any = None):
sfc-gh-bdufour marked this conversation as resolved.
Show resolved Hide resolved
with open(manifest_path, "r") as f:
manifest = safe_load(f)

version_info = manifest.setdefault("version", {})
version_info["name"] = version
if patch is not None:
version_info["patch"] = patch
else:
version_info.pop("patch", None)

with open(manifest_path, "w") as f:
f.write(safe_dump(manifest))


def normalize_identifier(identifier: Union[str, int]) -> str:
id_str = str(identifier)
Copy link
Collaborator

@sfc-gh-fcampbell sfc-gh-fcampbell Sep 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

according to the type this is already a str but I see that the test passes an int, so can we update the type?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops, updated.

if is_valid_unquoted_identifier(str(id_str)):
return id_str.upper()
else:
return unquote_identifier(to_identifier(id_str))


# Tests a simple flow of an existing project, executing snow app version create, drop and teardown, all with distribution=internal
@pytest.mark.integration
@pytest.mark.parametrize(
Expand Down Expand Up @@ -329,3 +360,78 @@ def test_nativeapp_version_create_package_no_magic_comment(
# Remove date field
row.pop("created_on", None)
assert actual.json == snapshot


# Tests a simple flow of an existing project, executing snow app version create, drop and teardown, all with distribution=internal
@pytest.mark.integration
@pytest.mark.parametrize(
"test_project",
[
"napp_init_v1",
"napp_init_v2",
],
)
def test_nativeapp_version_create_and_drop_from_manifest(
runner,
snowflake_session,
default_username,
resource_suffix,
nativeapp_project_directory,
test_project,
):
project_name = "myapp"
with nativeapp_project_directory(test_project) as project_dir:
# not using pytest parameterization here because we need
# to guarantee that the initial version gets created before the patches

VERSIONS = [7, "v1", "1.0", "version 1"]
PATCHES = [None, 10, "12"]

for version_name in VERSIONS:
manifest_path = project_dir / "app/manifest.yml"
set_version_in_app_manifest(manifest_path, version_name)

result_create = runner.invoke_with_connection_json(
["app", "version", "create", "--force", "--skip-git-check"]
)
assert result_create.exit_code == 0

# app package contains correct version
actual = runner.invoke_with_connection_json(["app", "version", "list"])
assert contains_row_with(
actual.json, {"version": normalize_identifier(version_name), "patch": 0}
)

result_drop = runner.invoke_with_connection_json(
["app", "version", "drop", "--force"]
)
assert result_drop.exit_code == 0
actual = runner.invoke_with_connection_json(["app", "version", "list"])
assert len(actual.json) == 0

version_name = "V2"
for patch_name in PATCHES:
manifest_path = project_dir / "app/manifest.yml"
set_version_in_app_manifest(manifest_path, version_name, patch_name)

result_create = runner.invoke_with_connection_json(
["app", "version", "create", "--force", "--skip-git-check"]
)
assert result_create.exit_code == 0

# app package contains version V2
actual = runner.invoke_with_connection_json(["app", "version", "list"])
assert contains_row_with(
actual.json,
{
"version": version_name,
"patch": int(patch_name) if patch_name else 0,
},
)

result_drop = runner.invoke_with_connection_json(
["app", "version", "drop", version_name, "--force"]
)
assert result_drop.exit_code == 0
actual = runner.invoke_with_connection_json(["app", "version", "list"])
assert len(actual.json) == 0
Loading