Add optional deploy files argument #933

sfc-gh-gbloom · 2024-03-25T15:21:17Z

Pre-review checklist

I've confirmed that instructions included in README.md are still correct after my changes in the codebase.
I've added or updated automated unit tests to verify correctness of my new code.
I've added or updated integration tests to verify correctness of my new code.
I've confirmed that my changes are working by executing CLI's commands manually.
I've confirmed that my changes are up-to-date with the target branch.
I've described my changes in the release notes.
I've described my changes in the section below.

Changes description

Following up on #921, this PR introduces the following changes:

The option to provide specific files and directories when running snow app deploy
The --prune/--no-prune option
The -r/--recursive option
Keep track of files copied/symlinked in the build step
Fix a bug in build_md5_map()

When determining the deploy path from source path, we use the mapping of the files created in the build step. Since we symlink entire directories, nested files will not exist in the mapping. In such cases, we traverse the parents chain until a mapped directory is found.

Usage

snow app deploy file1 dir1 dir2/file2 -r

If no files are specified, all files will be synced to stage (existing behavior).

The file paths are assumed to be relative to the project root.

Absolute paths, directories and patterns are currently not supported.

Pruning

When pruning is enabled, files that exist only on the stage will be removed.

When deploying all changes, --prune is the default value (existing behavior).
When deploying specific files, --no-prune is the default value and the user has to explicitly specify --prune to avoid accidentally removing files.

Recursive

Same as cp command, when specifying specific directories to deploy, the -r flag must be provided. When a directory is deployed, all of its content will be synced recursively.

When deploying all changes, -r is implied (existing behavior).

src/snowflake/cli/api/exceptions.py

src/snowflake/cli/plugins/nativeapp/commands.py

src/snowflake/cli/plugins/object/stage/diff.py

src/snowflake/cli/plugins/nativeapp/manager.py

src/snowflake/cli/plugins/object/stage/diff.py

sfc-gh-bdufour · 2024-03-26T18:57:10Z

An additional comment: your description states that the deploy root is CWD, but that's inaccurate, it's the project root that is current. Deploy root is typically a subdirectory of the project root. I still believe that falling back to look for files relative to the project root is a valuable addition to this command.

src/snowflake/cli/plugins/object/stage/diff.py

src/snowflake/cli/plugins/nativeapp/commands.py

src/snowflake/cli/plugins/object/stage/diff.py

sfc-gh-bgoel · 2024-04-29T13:10:26Z

src/snowflake/cli/plugins/nativeapp/artifacts.py

-                symlink_or_copy(source_path, dest_path / source_path.name)
+                dest_child_path = dest_path / source_path.name
+                symlink_or_copy(source_path, dest_child_path)
+                mapped_files[source_path.resolve()] = dest_child_path


Might be related to the discussion above, but why:

Is source_path being resolved?

Is only source_path being resolved but not dest_child_path?

Also, can you clarify the following:
If the user specified a glob pattern, then source_path will be the directories or files that qualify the source pattern. In case of directories, even though their contents are symlinked as well, mapped_files will only contain a kv pair of the directory to the dest, and not its contents.

Both src/dest parts in ArtifactDeploymentMap are resolved, mostly to canonicalize windows paths: C:\Users\RUNNER~1\ -> C:\Users\runneradmin\

dest_child_path is also resolved (dest_path is resolved a few lines above)

Yes, mapped_files won't contain the contents of symlinked directories. It's only the files that we actually copied to the deploy directory.
You can see the logic in project_path_to_deploy_path() for cases where we look for a file under a symlinked directory.

sfc-gh-bgoel · 2024-04-29T13:15:11Z

src/snowflake/cli/plugins/nativeapp/artifacts.py

+
+
+def project_path_to_deploy_path(
+    project_path: Path, mapped_files: ArtifactDeploymentMap


Nit: is this project_path or source_path? project_path can be misleading to only mean the path to the project, i.e. root of the project.

Agreed, I think source_path is a better name.

sfc-gh-bgoel · 2024-04-29T13:42:34Z

src/snowflake/cli/plugins/stage/diff.py

+    if prune:
+        diff.only_on_stage = [i for i in diff.only_on_stage if i in paths_to_sync]
+    else:
+        files_not_removed = [i for i in diff.only_on_stage if i in paths_to_sync]


Lines 167 and 169 rely on the same list comprehension. Consider assigning it to a placeholder/variable before checking for prune, to reduce duplicating code.

sfc-gh-cgorrie · 2024-04-29T15:20:57Z

src/snowflake/cli/plugins/nativeapp/artifacts.py

 from snowflake.cli.api.constants import DEFAULT_SIZE_LIMIT_MB
 from snowflake.cli.api.project.schemas.native_app.path_mapping import PathMapping
 from snowflake.cli.api.secure_path import SecurePath
 from yaml import safe_load

+# Map from source directories and files in the project directory to their path in the deploy directory. Both paths are absolute.


In an ideal world, these would be relative to their respective roots, but it's documented well and if we need them we can always use relative_to.

sfc-gh-cgorrie · 2024-04-29T15:21:42Z

src/snowflake/cli/plugins/nativeapp/artifacts.py

+
+
+def project_path_to_deploy_path(
+    project_path: Path, mapped_files: ArtifactDeploymentMap


Agreed, I think source_path is a better name.

src/snowflake/cli/plugins/nativeapp/artifacts.py

src/snowflake/cli/plugins/nativeapp/commands.py

src/snowflake/cli/plugins/nativeapp/manager.py

sfc-gh-mheavin

Left some thoughts for your consideration

sfc-gh-mheavin · 2024-04-29T17:06:40Z

RELEASE-NOTES.md

@@ -6,6 +6,7 @@

 ## New additions
 * `snow sql` command supports now client-side templating of queries.
+* Added the option to specify specific files and directories to sync when running `snow app deploy`.


Might be helpful to include the name of the option

sfc-gh-mheavin · 2024-04-29T17:11:40Z

src/snowflake/cli/plugins/nativeapp/artifacts.py

+def source_path_to_deploy_path(
+    source_path: Path, mapped_files: ArtifactDeploymentMap
+) -> Path:
+    """Given a source path and the files created during bundle, returns the absolute deploy destination path."""


Generally better to start with what it does.

Returns the absolute path where the specified application bundle was deployed. ??? (just guessing here)

sfc-gh-mheavin · 2024-04-29T17:12:53Z

src/snowflake/cli/plugins/nativeapp/artifacts.py

    """
    Prepares a local folder (deploy_root) with configured app artifacts.
    This folder can then be uploaded to a stage.
+    Returns a mapping of the copied source files, pointing to where they were copied to.


copied. (remove "to")

Also, is it a mapping or a map that gets returned? From the code, it looks like it returns a Map.

sfc-gh-mheavin · 2024-04-29T17:40:35Z

src/snowflake/cli/plugins/nativeapp/commands.py

@@ -229,17 +230,44 @@ def app_teardown(
 @app.command("deploy", requires_connection=True)
 @with_project_definition("native_app")
 def app_deploy(
+    prune: Optional[bool] = typer.Option(
+        default=None,
+        help=f"""If set, matched files that do not exist locally will also be deleted from the stage.""",


Whether to delete specified files from the stage if they don't exist locally. If set, the command deletes files that exist in the stage, but not in the local filesystem.

sfc-gh-mheavin · 2024-04-29T18:00:12Z

src/snowflake/cli/plugins/nativeapp/commands.py

+        None,
+        "--recursive",
+        "-r",
+        help=f"""Controls whether the specified directories should be deployed including all of their contents. If false, only the immediate files will be deployed, ignoring any sub-directories.""",


Whether to traverse and deploy files from subdirectories. If set, the command deploys all files and subdirectories; otherwise, only files in the current directory are deployed.

sfc-gh-mheavin · 2024-04-29T18:44:32Z

src/snowflake/cli/plugins/nativeapp/commands.py

+    files: Optional[List[Path]] = typer.Argument(
+        default=None,
+        show_default=False,
+        help=f"""Paths of the files, relative to the project root, to be uploaded to a stage. [default: sync all local changes to the stage]""",


Paths, relative to the the project root, of files you want to upload to a stage. If unspecified, the command syncs all local changes to the stage.

sfc-gh-mheavin · 2024-04-29T18:46:53Z

src/snowflake/cli/plugins/nativeapp/commands.py

    **options,
 ) -> CommandResult:
    """
    Creates an application package in your Snowflake account and syncs the local changes to the stage without creating or updating the application.
+    Running this command with no arguments at all is a shorthand for "snow app deploy --prune --recursive".


... at all, as in snow app deploy, ...

I think we use `` for commands. The scripts then convert it to the proper RST code format.

sfc-gh-mheavin · 2024-04-29T18:48:56Z

src/snowflake/cli/plugins/nativeapp/manager.py

@@ -100,6 +106,20 @@ def ensure_correct_owner(row: dict, role: str, obj_name: str) -> None:
        raise UnexpectedOwnerError(obj_name, role, actual_owner)


+def _get_paths_to_sync(paths_to_sync: List[Path], deploy_root: Path) -> List[str]:


Is this an internal function (so the doc string is not customer-visible)?

Yes, not visible to the customer using the CLI. But visible to the public in general since this is open source.
Regardless, good idea to still have some information on what this function does.

sfc-gh-turbaszek · 2024-05-06T15:05:51Z

src/snowflake/cli/plugins/nativeapp/commands.py

+        help=f"""Whether to delete specified files from the stage if they don't exist locally. If set, the command deletes files that exist in the stage, but not in the local filesystem.""",
+    ),
+    recursive: Optional[bool] = typer.Option(
+        None,


Personally I would use False as default for boolean value

I needed it to be None here because there is the special flow of running snow app deploy with no arguments, in which case the default value is True

* add deploy files argument * add --prune flag * add --recursive flag

sfc-gh-gbloom added 9 commits March 22, 2024 16:44

add files argument to deploy

ad43ea1

add integration test

041b385

stage files relative to deploy root

159752b

add unit tests

dc4c967

check for empty list

b95962d

Merge branch 'main' into gbloom-SNOW-1238239-deploy-files-argument

6265837

release notes update

2149053

update command description

f8b8c5e

unit test grooming

7a07388

sfc-gh-gbloom marked this pull request as ready for review March 26, 2024 17:26

sfc-gh-gbloom requested review from a team and sfc-gh-bgoel as code owners March 26, 2024 17:26

sfc-gh-bgoel reviewed Mar 26, 2024

View reviewed changes

src/snowflake/cli/api/exceptions.py Outdated Show resolved Hide resolved

src/snowflake/cli/plugins/nativeapp/commands.py Outdated Show resolved Hide resolved

src/snowflake/cli/plugins/object/stage/diff.py Outdated Show resolved Hide resolved

sfc-gh-bdufour reviewed Mar 26, 2024

View reviewed changes

sfc-gh-gbloom added 2 commits March 26, 2024 14:49

pr comments

9fa9f44

add types

1e29c4b

sfc-gh-cgorrie reviewed Mar 26, 2024

View reviewed changes

src/snowflake/cli/plugins/object/stage/diff.py Outdated Show resolved Hide resolved

sfc-gh-gbloom added 7 commits March 27, 2024 11:18

fix ignore only on stage files

e2119c3

update snapshot

5616e01

Merge branch 'main' into gbloom-SNOW-1238239-deploy-files-argument

9c4a902

fix set type for python3.8

3a1e46d

delete remote only files

29994b0

fix relative paths

f7963d8

add integration test of removing remote only file

1e82b1c

sfc-gh-gbloom requested review from sfc-gh-bdufour, sfc-gh-cgorrie and sfc-gh-bgoel March 27, 2024 21:28

sfc-gh-bgoel reviewed Mar 28, 2024

View reviewed changes

src/snowflake/cli/plugins/nativeapp/commands.py Outdated Show resolved Hide resolved

src/snowflake/cli/plugins/object/stage/diff.py Outdated Show resolved Hide resolved

sfc-gh-gbloom added 3 commits April 26, 2024 16:44

fix non recursive full deploy

f7bc13e

use different names in src/dest

b57f7d2

rename mapped_files

4568237

sfc-gh-bgoel reviewed Apr 29, 2024

View reviewed changes

sfc-gh-gbloom added 3 commits April 29, 2024 11:24

rename source_path

c7be70e

nit

fbd6b04

Merge branch 'main' into gbloom-SNOW-1238239-deploy-files-argument

a282972

sfc-gh-cgorrie reviewed Apr 29, 2024

View reviewed changes

sfc-gh-gbloom added 2 commits April 29, 2024 11:47

better docs

ef3dedd

update snapshot

4e4bf85

sfc-gh-mheavin reviewed Apr 29, 2024

View reviewed changes

sfc-gh-gbloom added 8 commits April 29, 2024 16:02

better docs

b8f0650

use ClickException

e1cc96d

tests

e3550de

Merge branch 'main' into gbloom-SNOW-1238239-deploy-files-argument

63db862

add snapshot

1ea1785

match click exception output

ece7214

remove text assertion

4e39f76

doc

be7d3a1

sfc-gh-cgorrie mentioned this pull request Apr 30, 2024

Fix the stage diff algorithm #1031

Merged

7 tasks

snapshot

19ab8a2

sfc-gh-cgorrie approved these changes Apr 30, 2024

View reviewed changes

Merge branch 'main' into gbloom-SNOW-1238239-deploy-files-argument

5b55216

sfc-gh-gbloom enabled auto-merge (squash) May 2, 2024 19:25

Merge branch 'main' into gbloom-SNOW-1238239-deploy-files-argument

dd293e9

sfc-gh-turbaszek reviewed May 6, 2024

View reviewed changes

sfc-gh-turbaszek approved these changes May 6, 2024

View reviewed changes

sfc-gh-gbloom merged commit 17138a6 into main May 6, 2024
11 checks passed

sfc-gh-gbloom deleted the gbloom-SNOW-1238239-deploy-files-argument branch May 6, 2024 15:07

sfc-gh-sichen pushed a commit that referenced this pull request Oct 17, 2024

Add optional deploy files argument (#933)

1fd948b

* add deploy files argument * add --prune flag * add --recursive flag

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add optional deploy files argument #933

Add optional deploy files argument #933

sfc-gh-gbloom commented Mar 25, 2024 •

edited

Loading

sfc-gh-bdufour commented Mar 26, 2024

sfc-gh-bgoel Apr 29, 2024

sfc-gh-gbloom Apr 29, 2024 •

edited

Loading

sfc-gh-bgoel Apr 29, 2024

sfc-gh-cgorrie Apr 29, 2024

sfc-gh-bgoel Apr 29, 2024

sfc-gh-cgorrie Apr 29, 2024

sfc-gh-cgorrie Apr 29, 2024

sfc-gh-mheavin left a comment

sfc-gh-mheavin Apr 29, 2024

sfc-gh-mheavin Apr 29, 2024

sfc-gh-mheavin Apr 29, 2024

sfc-gh-mheavin Apr 29, 2024

sfc-gh-mheavin Apr 29, 2024

sfc-gh-mheavin Apr 29, 2024

sfc-gh-mheavin Apr 29, 2024

sfc-gh-mheavin Apr 29, 2024

sfc-gh-bgoel Apr 29, 2024

sfc-gh-turbaszek May 6, 2024

sfc-gh-gbloom May 6, 2024



		def project_path_to_deploy_path(
		project_path: Path, mapped_files: ArtifactDeploymentMap

		@@ -100,6 +106,20 @@ def ensure_correct_owner(row: dict, role: str, obj_name: str) -> None:
		raise UnexpectedOwnerError(obj_name, role, actual_owner)


		def _get_paths_to_sync(paths_to_sync: List[Path], deploy_root: Path) -> List[str]:

Add optional deploy files argument #933

Add optional deploy files argument #933

Conversation

sfc-gh-gbloom commented Mar 25, 2024 • edited Loading

Pre-review checklist

Changes description

Usage

Pruning

Recursive

sfc-gh-bdufour commented Mar 26, 2024

Choose a reason for hiding this comment

sfc-gh-gbloom Apr 29, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sfc-gh-mheavin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sfc-gh-gbloom commented Mar 25, 2024 •

edited

Loading

sfc-gh-gbloom Apr 29, 2024 •

edited

Loading