From a061e2246faa6b292f6960681ed7f60be7c3ad4e Mon Sep 17 00:00:00 2001 From: Patryk Czajka Date: Tue, 27 Feb 2024 17:32:10 +0100 Subject: [PATCH 01/29] git copy: get --- src/snowflake/cli/api/utils/path_utils.py | 4 ++ src/snowflake/cli/plugins/git/commands.py | 42 +++++++++++++++++-- src/snowflake/cli/plugins/git/manager.py | 7 +++- .../cli/plugins/object/stage/commands.py | 9 ++-- 4 files changed, 51 insertions(+), 11 deletions(-) diff --git a/src/snowflake/cli/api/utils/path_utils.py b/src/snowflake/cli/api/utils/path_utils.py index bd05a003c4..9a090cf3e0 100644 --- a/src/snowflake/cli/api/utils/path_utils.py +++ b/src/snowflake/cli/api/utils/path_utils.py @@ -16,3 +16,7 @@ def path_resolver(path_to_file: str) -> str: if 0 < return_value <= BUFFER_SIZE: return buffer.value return path_to_file + + +def is_stage_path(path: str) -> bool: + return path.startswith("@") or path.startswith("snow://") diff --git a/src/snowflake/cli/plugins/git/commands.py b/src/snowflake/cli/plugins/git/commands.py index 240f2aea37..8f870b9a84 100644 --- a/src/snowflake/cli/plugins/git/commands.py +++ b/src/snowflake/cli/plugins/git/commands.py @@ -1,9 +1,11 @@ import logging +from pathlib import Path import typer from snowflake.cli.api.commands.flags import identifier_argument from snowflake.cli.api.commands.snow_typer import SnowTyper from snowflake.cli.api.output.types import CommandResult, QueryResult +from snowflake.cli.api.utils.path_utils import is_stage_path from snowflake.cli.plugins.git.manager import GitManager app = SnowTyper( @@ -14,6 +16,9 @@ log = logging.getLogger(__name__) RepoNameArgument = identifier_argument(sf_object="git repository", example="my_repo") +RepoPathArgument = typer.Argument( + help="Path to git repository stage with scope provided. For example: @my_repo/branches/main" +) @app.command( @@ -46,9 +51,7 @@ def list_tags( requires_connection=True, ) def list_files( - repository_path: str = typer.Argument( - help="Path to git repository stage with scope provided. For example: @my_repo/branches/main" - ), + repository_path: str = RepoPathArgument, **options, ) -> CommandResult: return QueryResult(GitManager().show_files(repo_path=repository_path)) @@ -64,3 +67,36 @@ def fetch( **options, ) -> CommandResult: return QueryResult(GitManager().fetch(repo_name=repository_name)) + + +@app.command( + "copy", + help="Copies all files from given state of repository to target directory.", + requires_connection=True, +) +def copy( + repository_path: str = RepoPathArgument, + destination_path: str = typer.Argument( + help="Target path for copy operation. Should be stage or local file path.", + ), + overwrite: bool = typer.Option( + False, + help="Overwrites existing files in the target path.", + ), + parallel: int = typer.Option( + 4, + help="Number of parallel threads to use when uploading files.", + ), + **options, +): + is_copy = is_stage_path(destination_path) + if is_copy: + cursor = GitManager.copy( + repo_path=repository_path, destination_path=destination_path + ) + else: + target = Path(destination_path).resolve() + cursor = GitManager().get( + stage_name=repository_path, dest_path=target, parallel=parallel + ) + return QueryResult(cursor) diff --git a/src/snowflake/cli/plugins/git/manager.py b/src/snowflake/cli/plugins/git/manager.py index 4cf357aab5..ea16bd2908 100644 --- a/src/snowflake/cli/plugins/git/manager.py +++ b/src/snowflake/cli/plugins/git/manager.py @@ -1,7 +1,7 @@ -from snowflake.cli.api.sql_execution import SqlExecutionMixin +from snowflake.cli.plugins.object.stage.manager import StageManager -class GitManager(SqlExecutionMixin): +class GitManager(StageManager): def show_branches(self, repo_name: str): query = f"show git branches in {repo_name}" return self._execute_query(query) @@ -17,3 +17,6 @@ def show_files(self, repo_path: str): def fetch(self, repo_name: str): query = f"alter git repository {repo_name} fetch" return self._execute_query(query) + + def copy(self, repo_path: str, destination_path: str): + pass diff --git a/src/snowflake/cli/plugins/object/stage/commands.py b/src/snowflake/cli/plugins/object/stage/commands.py index bf0540eca1..cf58395f86 100644 --- a/src/snowflake/cli/plugins/object/stage/commands.py +++ b/src/snowflake/cli/plugins/object/stage/commands.py @@ -11,6 +11,7 @@ QueryResult, SingleQueryResult, ) +from snowflake.cli.api.utils.path_utils import is_stage_path from snowflake.cli.plugins.object.stage.diff import DiffResult from snowflake.cli.plugins.object.stage.manager import StageManager @@ -31,10 +32,6 @@ def stage_list(stage_name: str = StageNameArgument, **options) -> CommandResult: return QueryResult(cursor) -def _is_stage_path(path: str): - return path.startswith("@") or path.startswith("snow://") - - @app.command("copy", requires_connection=True) def copy( source_path: str = typer.Argument( @@ -57,8 +54,8 @@ def copy( Copies all files from target path to target directory. This works for both uploading to and downloading files from the stage. """ - is_get = _is_stage_path(source_path) - is_put = _is_stage_path(destination_path) + is_get = is_stage_path(source_path) + is_put = is_stage_path(destination_path) if is_get and is_put: raise click.ClickException( From f3c10d980d967feebe48484a2544fe42fb3d9a78 Mon Sep 17 00:00:00 2001 From: Patryk Czajka Date: Tue, 27 Feb 2024 17:50:16 +0100 Subject: [PATCH 02/29] git copy: copy --- src/snowflake/cli/plugins/git/commands.py | 8 ++------ src/snowflake/cli/plugins/git/manager.py | 21 +++++++++++++++------ 2 files changed, 17 insertions(+), 12 deletions(-) diff --git a/src/snowflake/cli/plugins/git/commands.py b/src/snowflake/cli/plugins/git/commands.py index 8f870b9a84..fc73c33f3a 100644 --- a/src/snowflake/cli/plugins/git/commands.py +++ b/src/snowflake/cli/plugins/git/commands.py @@ -79,19 +79,15 @@ def copy( destination_path: str = typer.Argument( help="Target path for copy operation. Should be stage or local file path.", ), - overwrite: bool = typer.Option( - False, - help="Overwrites existing files in the target path.", - ), parallel: int = typer.Option( 4, - help="Number of parallel threads to use when uploading files.", + help="Number of parallel threads to use when downloading files.", ), **options, ): is_copy = is_stage_path(destination_path) if is_copy: - cursor = GitManager.copy( + cursor = GitManager().copy( repo_path=repository_path, destination_path=destination_path ) else: diff --git a/src/snowflake/cli/plugins/git/manager.py b/src/snowflake/cli/plugins/git/manager.py index ea16bd2908..05a4a02292 100644 --- a/src/snowflake/cli/plugins/git/manager.py +++ b/src/snowflake/cli/plugins/git/manager.py @@ -1,22 +1,31 @@ from snowflake.cli.plugins.object.stage.manager import StageManager +from snowflake.connector.cursor import SnowflakeCursor class GitManager(StageManager): - def show_branches(self, repo_name: str): + def show_branches(self, repo_name: str) -> SnowflakeCursor: query = f"show git branches in {repo_name}" return self._execute_query(query) - def show_tags(self, repo_name: str): + def show_tags(self, repo_name: str) -> SnowflakeCursor: query = f"show git tags in {repo_name}" return self._execute_query(query) - def show_files(self, repo_path: str): + def show_files(self, repo_path: str) -> SnowflakeCursor: query = f"ls {repo_path}" return self._execute_query(query) - def fetch(self, repo_name: str): + def fetch(self, repo_name: str) -> SnowflakeCursor: query = f"alter git repository {repo_name} fetch" return self._execute_query(query) - def copy(self, repo_path: str, destination_path: str): - pass + def _get_standard_stage_directory_path(self, path): + if not path.endswith("/"): + path += "/" + return self.get_standard_stage_name(path) + + def copy(self, repo_path: str, destination_path: str) -> SnowflakeCursor: + source = self._get_standard_stage_directory_path(repo_path) + destination = self._get_standard_stage_directory_path(destination_path) + query = f"copy files into {destination} from {source}" + return self._execute_query(query) From d9029e194e7732d75b3ecc8f01d8262d576638e1 Mon Sep 17 00:00:00 2001 From: Patryk Czajka Date: Tue, 27 Feb 2024 18:00:40 +0100 Subject: [PATCH 03/29] unit tests --- src/snowflake/cli/plugins/git/commands.py | 2 +- tests/__snapshots__/test_help_messages.ambr | 84 ++++++++++++++++++++- tests/git/test_git_commands.py | 28 +++++++ 3 files changed, 109 insertions(+), 5 deletions(-) diff --git a/src/snowflake/cli/plugins/git/commands.py b/src/snowflake/cli/plugins/git/commands.py index fc73c33f3a..c4804fa6ee 100644 --- a/src/snowflake/cli/plugins/git/commands.py +++ b/src/snowflake/cli/plugins/git/commands.py @@ -71,7 +71,7 @@ def fetch( @app.command( "copy", - help="Copies all files from given state of repository to target directory.", + help="Copies all files from given state of repository to local directory or stage.", requires_connection=True, ) def copy( diff --git a/tests/__snapshots__/test_help_messages.ambr b/tests/__snapshots__/test_help_messages.ambr index d3afa2d613..ec3e75b72c 100644 --- a/tests/__snapshots__/test_help_messages.ambr +++ b/tests/__snapshots__/test_help_messages.ambr @@ -751,6 +751,80 @@ ╰──────────────────────────────────────────────────────────────────────────────╯ + ''' +# --- +# name: test_help_messages[git.copy] + ''' + + Usage: default git copy [OPTIONS] REPOSITORY_PATH DESTINATION_PATH + + Copies all files from given state of repository to local directory or stage. + + ╭─ Arguments ──────────────────────────────────────────────────────────────────╮ + │ * repository_path TEXT Path to git repository stage with scope │ + │ provided. For example: │ + │ @my_repo/branches/main │ + │ [default: None] │ + │ [required] │ + │ * destination_path TEXT Target path for copy operation. Should be │ + │ stage or local file path. │ + │ [default: None] │ + │ [required] │ + ╰──────────────────────────────────────────────────────────────────────────────╯ + ╭─ Options ────────────────────────────────────────────────────────────────────╮ + │ --parallel INTEGER Number of parallel threads to use when │ + │ downloading files. │ + │ [default: 4] │ + │ --help -h Show this message and exit. │ + ╰──────────────────────────────────────────────────────────────────────────────╯ + ╭─ Connection configuration ───────────────────────────────────────────────────╮ + │ --connection,--environment -c TEXT Name of the connection, as defined │ + │ in your `config.toml`. Default: │ + │ `default`. │ + │ --account,--accountname TEXT Name assigned to your Snowflake │ + │ account. Overrides the value │ + │ specified for the connection. │ + │ --user,--username TEXT Username to connect to Snowflake. │ + │ Overrides the value specified for │ + │ the connection. │ + │ --password TEXT Snowflake password. Overrides the │ + │ value specified for the │ + │ connection. │ + │ --authenticator TEXT Snowflake authenticator. Overrides │ + │ the value specified for the │ + │ connection. │ + │ --private-key-path TEXT Snowflake private key path. │ + │ Overrides the value specified for │ + │ the connection. │ + │ --database,--dbname TEXT Database to use. Overrides the │ + │ value specified for the │ + │ connection. │ + │ --schema,--schemaname TEXT Database schema to use. Overrides │ + │ the value specified for the │ + │ connection. │ + │ --role,--rolename TEXT Role to use. Overrides the value │ + │ specified for the connection. │ + │ --warehouse TEXT Warehouse to use. Overrides the │ + │ value specified for the │ + │ connection. │ + │ --temporary-connection -x Uses connection defined with │ + │ command line parameters, instead │ + │ of one defined in config │ + │ --mfa-passcode TEXT Token to use for multi-factor │ + │ authentication (MFA) │ + ╰──────────────────────────────────────────────────────────────────────────────╯ + ╭─ Global configuration ───────────────────────────────────────────────────────╮ + │ --format [TABLE|JSON] Specifies the output format. │ + │ [default: TABLE] │ + │ --verbose -v Displays log entries for log levels `info` │ + │ and higher. │ + │ --debug Displays log entries for log levels `debug` │ + │ and higher; debug logs contains additional │ + │ information. │ + │ --silent Turns off intermediate output to console. │ + ╰──────────────────────────────────────────────────────────────────────────────╯ + + ''' # --- # name: test_help_messages[git.fetch] @@ -1029,10 +1103,12 @@ │ --help -h Show this message and exit. │ ╰──────────────────────────────────────────────────────────────────────────────╯ ╭─ Commands ───────────────────────────────────────────────────────────────────╮ - │ fetch Fetch changes from origin to snowflake repository. │ - │ list-branches List all branches in the repository. │ - │ list-files List files from given state of git repository. │ - │ list-tags List all tags in the repository. │ + │ copy Copies all files from given state of repository to local │ + │ directory or stage. │ + │ fetch Fetch changes from origin to snowflake repository. │ + │ list-branches List all branches in the repository. │ + │ list-files List files from given state of git repository. │ + │ list-tags List all tags in the repository. │ ╰──────────────────────────────────────────────────────────────────────────────╯ diff --git a/tests/git/test_git_commands.py b/tests/git/test_git_commands.py index 0a7b0c42f4..bb159007aa 100644 --- a/tests/git/test_git_commands.py +++ b/tests/git/test_git_commands.py @@ -1,3 +1,4 @@ +from pathlib import Path from unittest import mock import pytest @@ -53,3 +54,30 @@ def test_fetch(mock_connector, runner, mock_ctx): assert result.exit_code == 0, result.output assert ctx.get_query() == "alter git repository repo_name fetch" + + +@mock.patch("snowflake.connector.connect") +def test_copy(mock_connector, runner, mock_ctx): + ctx = mock_ctx() + mock_connector.return_value = ctx + local_path = Path("local/path") + result = runner.invoke(["git", "copy", "@repo_name/branches/main", str(local_path)]) + + assert result.exit_code == 0, result.output + assert ( + ctx.get_query() + == f"get @repo_name/branches/main file://{local_path.resolve()}/ parallel=4" + ) + + ctx = mock_ctx() + mock_connector.return_value = ctx + result = runner.invoke( + ["git", "copy", "@repo_name/branches/main", "@stage_path/dir_in_stage"] + ) + + # paths in generated SQL should end with '/' + assert result.exit_code == 0, result.output + assert ( + ctx.get_query() + == "copy files into @stage_path/dir_in_stage/ from @repo_name/branches/main/" + ) From db62c6cb6c414e0a4bb59446681c2dd54658fa63 Mon Sep 17 00:00:00 2001 From: Patryk Czajka Date: Wed, 28 Feb 2024 14:46:37 +0100 Subject: [PATCH 04/29] Setting up api integration --- tests_integration/test_git.py | 53 +++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) create mode 100644 tests_integration/test_git.py diff --git a/tests_integration/test_git.py b/tests_integration/test_git.py new file mode 100644 index 0000000000..01d9db078e --- /dev/null +++ b/tests_integration/test_git.py @@ -0,0 +1,53 @@ +import time +from unittest import mock + +import pytest + + +@pytest.fixture +def git_repository(runner): + repo_name = "GITHUB_SNOWCLI_API_INTEGRATION" + integration_name = "GITHUB_SNOWCLI_API_INTEGRATION" + + if not _integration_exists(runner, integration_name=integration_name): + result = runner.invoke_with_connection( + [ + "sql", + "-q", + f""" + CREATE API INTEGRATION {integration_name} + API_PROVIDER = git_https_api + API_ALLOWED_PREFIXES = ('https://github.com/Snowflake-Labs') + ALLOWED_AUTHENTICATION_SECRETS = () + ENABLED = true + """, + ] + ) + assert result.exit_code == 0 + + result = runner.invoke_with_connection( + [ + "sql", + "-q", + f""" + CREATE GIT REPOSITORY {repo_name} + API_INTEGRATION = {integration_name} + ORIGIN = 'https://github.com/Snowflake-Labs/snowflake-cli.git' + """, + ] + ) + assert result.exit_code == 0 + return repo_name + + +@pytest.mark.integration +def test_flow(runner, git_repository): + result = runner.invoke_with_connection(["object", "list", "git-repository"]) + print(result.output) + assert result.exit_code == 0 + + +def _integration_exists(runner, integration_name): + result = runner.invoke_with_connection_json(["sql", "-q", "SHOW INTEGRATIONS"]) + assert result.exit_code == 0 + return any(integration["name"] == integration_name for integration in result.json) From eaf972db868f0ce81a236ce601c37112dcfc229d Mon Sep 17 00:00:00 2001 From: Patryk Czajka Date: Wed, 28 Feb 2024 16:53:13 +0100 Subject: [PATCH 05/29] test git flow - in progress --- tests_integration/test_git.py | 89 +++++++++++++++++++++++++++++++---- 1 file changed, 80 insertions(+), 9 deletions(-) diff --git a/tests_integration/test_git.py b/tests_integration/test_git.py index 01d9db078e..66f693d440 100644 --- a/tests_integration/test_git.py +++ b/tests_integration/test_git.py @@ -1,11 +1,10 @@ -import time -from unittest import mock - import pytest +from click import ClickException + @pytest.fixture -def git_repository(runner): +def git_repository(runner, test_database): repo_name = "GITHUB_SNOWCLI_API_INTEGRATION" integration_name = "GITHUB_SNOWCLI_API_INTEGRATION" @@ -41,13 +40,85 @@ def git_repository(runner): @pytest.mark.integration -def test_flow(runner, git_repository): - result = runner.invoke_with_connection(["object", "list", "git-repository"]) - print(result.output) - assert result.exit_code == 0 +def test_flow(runner, git_repository, snapshot): + if False: + # object list + result = runner.invoke_with_connection_json( + ["object", "list", "git-repository"] + ) + assert result.exit_code == 0 + _assert_object_with_name_in(result.json, name=git_repository) + + # describe + result = runner.invoke_with_connection_json( + ["object", "describe", "git-repository", git_repository] + ) + assert result.exit_code == 0 + assert result.json[0]["name"] == git_repository + + # fetch + result = runner.invoke_with_connection_json(["git", "fetch", git_repository]) + assert result.exit_code == 0 + assert result.json == [ + { + "status": f"Git Repository {git_repository} is up to date. No change was fetched." + } + ] + + # list branches + result = runner.invoke_with_connection_json( + ["git", "list-branches", git_repository] + ) + assert result.exit_code == 0 + _assert_object_with_name_in(result.json, name="main") + + # list tags + result = runner.invoke_with_connection_json( + ["git", "list-tags", git_repository] + ) + assert result.exit_code == 0 + _assert_object_with_name_in(result.json, name="v2.0.0rc2") + + # list files - error messages + result = runner.invoke_with_connection(["git", "list-files", git_repository]) + assert ( + result.exit_code == 1 + and result.output == snapshot["list-files-not-stage-error"] + ) + result = runner.invoke_with_connection(["git", "list-files"]) + + repository_path = f"@{git_repository}" + result = runner.invoke_with_connection(["git", "list-files", repository_path]) + print(result.__dict__) + + repository_path = f"@{git_repository}/branches/beach_which_does_not_exist" + result = runner.invoke_with_connection(["git", "list-files", repository_path]) + print(result.__dict__) + + repository_path = f"@{git_repository}/tags/tag_which_does_not_exist" + result = runner.invoke_with_connection(["git", "list-files", repository_path]) + print(result.__dict__) + + repository_path = f"@{git_repository}/branches/main" + result = runner.invoke_with_connection_json(["git", "list-files", repository_path]) + # _assert_object_with_name_in() + + # for later + # copy - error messages + result = runner.invoke_with_connection( + ["git", "copy", git_repository, "@some_stage"] + ) + assert result.exit_code == 1 and result.output == snapshot["copy-not-stage-error"] + + +def _assert_object_with_name_in(objects, *, name): + assert any(object["name"] == name for object in objects) def _integration_exists(runner, integration_name): result = runner.invoke_with_connection_json(["sql", "-q", "SHOW INTEGRATIONS"]) assert result.exit_code == 0 - return any(integration["name"] == integration_name for integration in result.json) + return any( + integration["name"].upper() == integration_name.upper() + for integration in result.json + ) From e7661b667fd4e107195feb8357bea76b222495f7 Mon Sep 17 00:00:00 2001 From: Patryk Czajka Date: Wed, 28 Feb 2024 17:10:09 +0100 Subject: [PATCH 06/29] copy: fix error handling --- src/snowflake/cli/plugins/git/commands.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/snowflake/cli/plugins/git/commands.py b/src/snowflake/cli/plugins/git/commands.py index c4804fa6ee..2dd5e714b1 100644 --- a/src/snowflake/cli/plugins/git/commands.py +++ b/src/snowflake/cli/plugins/git/commands.py @@ -2,6 +2,7 @@ from pathlib import Path import typer +from click import ClickException from snowflake.cli.api.commands.flags import identifier_argument from snowflake.cli.api.commands.snow_typer import SnowTyper from snowflake.cli.api.output.types import CommandResult, QueryResult @@ -54,6 +55,7 @@ def list_files( repository_path: str = RepoPathArgument, **options, ) -> CommandResult: + _assert_repository_path_is_stage("REPOSITORY_PATH", path=repository_path) return QueryResult(GitManager().show_files(repo_path=repository_path)) @@ -85,6 +87,7 @@ def copy( ), **options, ): + _assert_repository_path_is_stage("REPOSITORY_PATH", path=repository_path) is_copy = is_stage_path(destination_path) if is_copy: cursor = GitManager().copy( @@ -96,3 +99,11 @@ def copy( stage_name=repository_path, dest_path=target, parallel=parallel ) return QueryResult(cursor) + + +def _assert_repository_path_is_stage(argument_name, path): + if not is_stage_path(path): + raise ClickException( + f"{argument_name} should be a path to git repository stage with scope provided." + " For example: @my_repo/branches/main" + ) From a9b6236cfcb3abf32dd1ee5f37b15191ed62bfc0 Mon Sep 17 00:00:00 2001 From: Patryk Czajka Date: Fri, 1 Mar 2024 17:16:14 +0100 Subject: [PATCH 07/29] Add unit test for checking error messages --- tests/git/test_git_commands.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/tests/git/test_git_commands.py b/tests/git/test_git_commands.py index bb159007aa..be252284b8 100644 --- a/tests/git/test_git_commands.py +++ b/tests/git/test_git_commands.py @@ -45,6 +45,11 @@ def test_list_files(mock_connector, runner, mock_ctx): assert result.exit_code == 0, result.output assert ctx.get_query() == "ls @repo_name/branches/main" + # detect whether repo_path is a stage path + result = runner.invoke(["git", "list-files", "not_a_stage_path"]) + assert result.exit_code == 1 + _assert_error_message(result.output) + @mock.patch("snowflake.connector.connect") def test_fetch(mock_connector, runner, mock_ctx): @@ -81,3 +86,16 @@ def test_copy(mock_connector, runner, mock_ctx): ctx.get_query() == "copy files into @stage_path/dir_in_stage/ from @repo_name/branches/main/" ) + + # detect whether repo_path is a stage path + result = runner.invoke(["git", "list-files", "not_a_stage_path"]) + assert result.exit_code == 1 + _assert_error_message(result.output) + + +def _assert_error_message(output): + assert "Error" in output + assert ( + "REPOSITORY_PATH should be a path to git repository stage with scope" in output + ) + assert "provided. For example: @my_repo/branches/main" in output From 228fb3e58911c94d76cf4fa9e1476ecf23a7cc7f Mon Sep 17 00:00:00 2001 From: Patryk Czajka Date: Fri, 1 Mar 2024 17:38:32 +0100 Subject: [PATCH 08/29] finish flow test --- tests_integration/__snapshots__/test_git.ambr | 10 + tests_integration/test_git.py | 189 ++++++++++++------ 2 files changed, 141 insertions(+), 58 deletions(-) create mode 100644 tests_integration/__snapshots__/test_git.ambr diff --git a/tests_integration/__snapshots__/test_git.ambr b/tests_integration/__snapshots__/test_git.ambr new file mode 100644 index 0000000000..4ce35c50d9 --- /dev/null +++ b/tests_integration/__snapshots__/test_git.ambr @@ -0,0 +1,10 @@ +# serializer version: 1 +# name: test_flow[copy-not-stage-error] + ''' + ╭─ Error ──────────────────────────────────────────────────────────────────────╮ + │ REPOSITORY_PATH should be a path to git repository stage with scope │ + │ provided. For example: @my_repo/branches/main │ + ╰──────────────────────────────────────────────────────────────────────────────╯ + + ''' +# --- diff --git a/tests_integration/test_git.py b/tests_integration/test_git.py index 66f693d440..cf5db70814 100644 --- a/tests_integration/test_git.py +++ b/tests_integration/test_git.py @@ -1,6 +1,7 @@ import pytest - -from click import ClickException +from snowflake.connector.errors import ProgrammingError +from pathlib import Path +import tempfile @pytest.fixture @@ -41,78 +42,150 @@ def git_repository(runner, test_database): @pytest.mark.integration def test_flow(runner, git_repository, snapshot): - if False: - # object list - result = runner.invoke_with_connection_json( - ["object", "list", "git-repository"] - ) - assert result.exit_code == 0 - _assert_object_with_name_in(result.json, name=git_repository) - - # describe - result = runner.invoke_with_connection_json( - ["object", "describe", "git-repository", git_repository] - ) - assert result.exit_code == 0 - assert result.json[0]["name"] == git_repository + # object list + result = runner.invoke_with_connection_json(["object", "list", "git-repository"]) + assert result.exit_code == 0 + _assert_object_with_key_value(result.json, key="name", value=git_repository) - # fetch - result = runner.invoke_with_connection_json(["git", "fetch", git_repository]) - assert result.exit_code == 0 - assert result.json == [ - { - "status": f"Git Repository {git_repository} is up to date. No change was fetched." - } - ] + # describe + result = runner.invoke_with_connection_json( + ["object", "describe", "git-repository", git_repository] + ) + assert result.exit_code == 0 + assert result.json[0]["name"] == git_repository - # list branches - result = runner.invoke_with_connection_json( - ["git", "list-branches", git_repository] - ) - assert result.exit_code == 0 - _assert_object_with_name_in(result.json, name="main") + # fetch + result = runner.invoke_with_connection_json(["git", "fetch", git_repository]) + assert result.exit_code == 0 + assert result.json == [ + { + "status": f"Git Repository {git_repository} is up to date. No change was fetched." + } + ] + + # list branches + result = runner.invoke_with_connection_json( + ["git", "list-branches", git_repository] + ) + assert result.exit_code == 0 + _assert_object_with_key_value(result.json, key="name", value="main") - # list tags - result = runner.invoke_with_connection_json( - ["git", "list-tags", git_repository] - ) - assert result.exit_code == 0 - _assert_object_with_name_in(result.json, name="v2.0.0rc2") + # list tags + result = runner.invoke_with_connection_json(["git", "list-tags", git_repository]) + assert result.exit_code == 0 + _assert_object_with_key_value(result.json, key="name", value="v2.0.0rc2") # list files - error messages result = runner.invoke_with_connection(["git", "list-files", git_repository]) - assert ( - result.exit_code == 1 - and result.output == snapshot["list-files-not-stage-error"] - ) - result = runner.invoke_with_connection(["git", "list-files"]) - - repository_path = f"@{git_repository}" - result = runner.invoke_with_connection(["git", "list-files", repository_path]) - print(result.__dict__) + assert result.output == snapshot(name="list-files-not-stage-error") + + try: + repository_path = f"@{git_repository}" + runner.invoke_with_connection(["git", "list-files", repository_path]) + except ProgrammingError as err: + assert ( + err.raw_msg + == "Files paths in git repositories must specify a scope. For example, a branch name, " + "a tag name, or a valid commit hash. Commit hashes are between 6 and 40 characters long." + ) - repository_path = f"@{git_repository}/branches/beach_which_does_not_exist" - result = runner.invoke_with_connection(["git", "list-files", repository_path]) - print(result.__dict__) + try: + repository_path = f"@{git_repository}/branches/branch_which_does_not_exist" + runner.invoke_with_connection(["git", "list-files", repository_path]) + except ProgrammingError as err: + assert ( + err.raw_msg + == "The specified branch 'branch_which_does_not_exist' cannot be found in the Git Repository." + ) - repository_path = f"@{git_repository}/tags/tag_which_does_not_exist" - result = runner.invoke_with_connection(["git", "list-files", repository_path]) - print(result.__dict__) + try: + repository_path = f"@{git_repository}/tags/tag_which_does_not_exist" + runner.invoke_with_connection(["git", "list-files", repository_path]) + except ProgrammingError as err: + assert ( + err.raw_msg + == "The specified tag 'tag_which_does_not_exist' cannot be found in the Git Repository." + ) + # list-files - success repository_path = f"@{git_repository}/branches/main" result = runner.invoke_with_connection_json(["git", "list-files", repository_path]) - # _assert_object_with_name_in() + _assert_object_with_key_value( + result.json, + key="name", + value="github_snowcli_api_integration/branches/main/RELEASE-NOTES.md", + ) + + # create stage for testing copy + stage_name = "a_perfect_stage_for_testing" + result = runner.invoke_with_connection(["sql", "-q", f"create stage {stage_name}"]) - # for later - # copy - error messages + # copy - test error messages result = runner.invoke_with_connection( - ["git", "copy", git_repository, "@some_stage"] + ["git", "copy", git_repository, f"@{stage_name}"] + ) + assert result.output == snapshot(name="copy-not-stage-error") + try: + repository_path = f"@{git_repository}" + runner.invoke_with_connection( + ["git", "copy", repository_path, f"@{stage_name}"] + ) + except ProgrammingError as err: + assert ( + err.raw_msg + == "Files paths in git repositories must specify a scope. For example, a branch name, " + "a tag name, or a valid commit hash. Commit hashes are between 6 and 40 characters long." + ) + + try: + repository_path = f"@{git_repository}/branches/branch_which_does_not_exist" + runner.invoke_with_connection( + ["git", "copy", repository_path, f"@{stage_name}"] + ) + except ProgrammingError as err: + assert ( + err.raw_msg + == "The specified branch 'branch_which_does_not_exist' cannot be found in the Git Repository." + ) + + try: + repository_path = f"@{git_repository}/tags/tag_which_does_not_exist" + runner.invoke_with_connection( + ["git", "copy", repository_path, f"@{stage_name}"] + ) + except ProgrammingError as err: + assert ( + err.raw_msg + == "The specified tag 'tag_which_does_not_exist' cannot be found in the Git Repository." + ) + + # copy to stage - success + repository_path = f"@{git_repository}/branches/main" + result = runner.invoke_with_connection_json( + ["git", "copy", repository_path, f"@{stage_name}"] + ) + assert result.exit_code == 0 + _assert_object_with_key_value(result.json, key="file", value="RELEASE-NOTES.md") + result = runner.invoke_with_connection_json(["object", "stage", "list", stage_name]) + _assert_object_with_key_value( + result.json, key="name", value=f"{stage_name}/RELEASE-NOTES.md" ) - assert result.exit_code == 1 and result.output == snapshot["copy-not-stage-error"] + + with tempfile.TemporaryDirectory() as tmp_dir: + # copy to local file system - success + repository_path = f"@{git_repository}/branches/main/" + result = runner.invoke_with_connection_json( + ["git", "copy", repository_path, tmp_dir] + ) + assert result.exit_code == 0 + assert (Path(tmp_dir) / "RELEASE-NOTES.md").exists() + import json + + print(json.dumps(result.json, indent=2)) -def _assert_object_with_name_in(objects, *, name): - assert any(object["name"] == name for object in objects) +def _assert_object_with_key_value(objects, *, key, value): + assert any(object[key] == value for object in objects) def _integration_exists(runner, integration_name): From 601cd75293da6f44d7eb41fb2dfee17df9f3ebf1 Mon Sep 17 00:00:00 2001 From: Patryk Czajka Date: Mon, 4 Mar 2024 10:21:41 +0100 Subject: [PATCH 09/29] fix get command --- src/snowflake/cli/plugins/git/commands.py | 2 +- src/snowflake/cli/plugins/git/manager.py | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/src/snowflake/cli/plugins/git/commands.py b/src/snowflake/cli/plugins/git/commands.py index 2dd5e714b1..d341105ef3 100644 --- a/src/snowflake/cli/plugins/git/commands.py +++ b/src/snowflake/cli/plugins/git/commands.py @@ -96,7 +96,7 @@ def copy( else: target = Path(destination_path).resolve() cursor = GitManager().get( - stage_name=repository_path, dest_path=target, parallel=parallel + repo_path=repository_path, target_path=target, parallel=parallel ) return QueryResult(cursor) diff --git a/src/snowflake/cli/plugins/git/manager.py b/src/snowflake/cli/plugins/git/manager.py index 05a4a02292..aa1fd9c365 100644 --- a/src/snowflake/cli/plugins/git/manager.py +++ b/src/snowflake/cli/plugins/git/manager.py @@ -1,3 +1,5 @@ +from pathlib import Path + from snowflake.cli.plugins.object.stage.manager import StageManager from snowflake.connector.cursor import SnowflakeCursor @@ -24,6 +26,13 @@ def _get_standard_stage_directory_path(self, path): path += "/" return self.get_standard_stage_name(path) + def get(self, repo_path: str, target_path: Path, parallel: int) -> SnowflakeCursor: + return super().get( + stage_name=self._get_standard_stage_directory_path(repo_path), + dest_path=target_path, + parallel=parallel, + ) + def copy(self, repo_path: str, destination_path: str) -> SnowflakeCursor: source = self._get_standard_stage_directory_path(repo_path) destination = self._get_standard_stage_directory_path(destination_path) From 5be18d81e63745c97ca5434f93cf60ee1afee1ea Mon Sep 17 00:00:00 2001 From: Patryk Czajka Date: Mon, 4 Mar 2024 13:15:12 +0100 Subject: [PATCH 10/29] switch to smaller repo --- tests_integration/__snapshots__/test_git.ambr | 2 +- tests_integration/test_git.py | 56 ++++++++++++------- 2 files changed, 37 insertions(+), 21 deletions(-) diff --git a/tests_integration/__snapshots__/test_git.ambr b/tests_integration/__snapshots__/test_git.ambr index 4ce35c50d9..b49d4ce029 100644 --- a/tests_integration/__snapshots__/test_git.ambr +++ b/tests_integration/__snapshots__/test_git.ambr @@ -1,5 +1,5 @@ # serializer version: 1 -# name: test_flow[copy-not-stage-error] +# name: test_flow[list-files-not-stage-error] ''' ╭─ Error ──────────────────────────────────────────────────────────────────────╮ │ REPOSITORY_PATH should be a path to git repository stage with scope │ diff --git a/tests_integration/test_git.py b/tests_integration/test_git.py index cf5db70814..7b0657bff4 100644 --- a/tests_integration/test_git.py +++ b/tests_integration/test_git.py @@ -6,8 +6,8 @@ @pytest.fixture def git_repository(runner, test_database): - repo_name = "GITHUB_SNOWCLI_API_INTEGRATION" - integration_name = "GITHUB_SNOWCLI_API_INTEGRATION" + repo_name = "SNOWCLI_DUMMY_REPO" + integration_name = "SNOWCLI_DUMMY_REPO_API_INTEGRATION" if not _integration_exists(runner, integration_name=integration_name): result = runner.invoke_with_connection( @@ -17,7 +17,7 @@ def git_repository(runner, test_database): f""" CREATE API INTEGRATION {integration_name} API_PROVIDER = git_https_api - API_ALLOWED_PREFIXES = ('https://github.com/Snowflake-Labs') + API_ALLOWED_PREFIXES = ('https://github.com/sfc-gh-pczajka') ALLOWED_AUTHENTICATION_SECRETS = () ENABLED = true """, @@ -32,7 +32,7 @@ def git_repository(runner, test_database): f""" CREATE GIT REPOSITORY {repo_name} API_INTEGRATION = {integration_name} - ORIGIN = 'https://github.com/Snowflake-Labs/snowflake-cli.git' + ORIGIN = 'https://github.com/sfc-gh-pczajka/dummy-repo-for-snowcli-testing.git' """, ] ) @@ -42,6 +42,9 @@ def git_repository(runner, test_database): @pytest.mark.integration def test_flow(runner, git_repository, snapshot): + TAG_NAME = "a-particular-tag" + BRANCH_NAME = "a-branch-which-is-different-than-main" + # object list result = runner.invoke_with_connection_json(["object", "list", "git-repository"]) assert result.exit_code == 0 @@ -69,11 +72,12 @@ def test_flow(runner, git_repository, snapshot): ) assert result.exit_code == 0 _assert_object_with_key_value(result.json, key="name", value="main") + _assert_object_with_key_value(result.json, key="name", value=BRANCH_NAME) # list tags result = runner.invoke_with_connection_json(["git", "list-tags", git_repository]) assert result.exit_code == 0 - _assert_object_with_key_value(result.json, key="name", value="v2.0.0rc2") + _assert_object_with_key_value(result.json, key="name", value=TAG_NAME) # list files - error messages result = runner.invoke_with_connection(["git", "list-files", git_repository]) @@ -108,27 +112,35 @@ def test_flow(runner, git_repository, snapshot): ) # list-files - success - repository_path = f"@{git_repository}/branches/main" + repository_path = f"@{git_repository}/branches/{BRANCH_NAME}" + result = runner.invoke_with_connection_json(["git", "list-files", repository_path]) + _assert_object_with_key_value( + result.json, + key="name", + value=f"{repository_path[1:]}/file_only_present_on_a_different_branch.txt", + ) + repository_path = f"@{git_repository}/tags/{TAG_NAME}" result = runner.invoke_with_connection_json(["git", "list-files", repository_path]) _assert_object_with_key_value( result.json, key="name", - value="github_snowcli_api_integration/branches/main/RELEASE-NOTES.md", + value=f"{repository_path[1:]}/file_only_present_on_a_particular_tag.txt", ) # create stage for testing copy - stage_name = "a_perfect_stage_for_testing" - result = runner.invoke_with_connection(["sql", "-q", f"create stage {stage_name}"]) + STAGE_NAME = "a_perfect_stage_for_testing" + result = runner.invoke_with_connection(["sql", "-q", f"create stage {STAGE_NAME}"]) + assert result.exit_code == 0 # copy - test error messages result = runner.invoke_with_connection( - ["git", "copy", git_repository, f"@{stage_name}"] + ["git", "copy", git_repository, f"@{STAGE_NAME}"] ) assert result.output == snapshot(name="copy-not-stage-error") try: repository_path = f"@{git_repository}" runner.invoke_with_connection( - ["git", "copy", repository_path, f"@{stage_name}"] + ["git", "copy", repository_path, f"@{STAGE_NAME}"] ) except ProgrammingError as err: assert ( @@ -140,7 +152,7 @@ def test_flow(runner, git_repository, snapshot): try: repository_path = f"@{git_repository}/branches/branch_which_does_not_exist" runner.invoke_with_connection( - ["git", "copy", repository_path, f"@{stage_name}"] + ["git", "copy", repository_path, f"@{STAGE_NAME}"] ) except ProgrammingError as err: assert ( @@ -151,7 +163,7 @@ def test_flow(runner, git_repository, snapshot): try: repository_path = f"@{git_repository}/tags/tag_which_does_not_exist" runner.invoke_with_connection( - ["git", "copy", repository_path, f"@{stage_name}"] + ["git", "copy", repository_path, f"@{STAGE_NAME}"] ) except ProgrammingError as err: assert ( @@ -160,25 +172,29 @@ def test_flow(runner, git_repository, snapshot): ) # copy to stage - success - repository_path = f"@{git_repository}/branches/main" + repository_path = f"@{git_repository}/branches/{BRANCH_NAME}" result = runner.invoke_with_connection_json( - ["git", "copy", repository_path, f"@{stage_name}"] + ["git", "copy", repository_path, f"@{STAGE_NAME}"] ) assert result.exit_code == 0 - _assert_object_with_key_value(result.json, key="file", value="RELEASE-NOTES.md") - result = runner.invoke_with_connection_json(["object", "stage", "list", stage_name]) _assert_object_with_key_value( - result.json, key="name", value=f"{stage_name}/RELEASE-NOTES.md" + result.json, key="file", value="file_only_present_on_a_different_branch.txt" + ) + result = runner.invoke_with_connection_json(["object", "stage", "list", STAGE_NAME]) + _assert_object_with_key_value( + result.json, + key="name", + value=f"{STAGE_NAME}/file_only_present_on_a_different_branch.txt", ) with tempfile.TemporaryDirectory() as tmp_dir: # copy to local file system - success - repository_path = f"@{git_repository}/branches/main/" + repository_path = f"@{git_repository}/tags/{TAG_NAME}" result = runner.invoke_with_connection_json( ["git", "copy", repository_path, tmp_dir] ) assert result.exit_code == 0 - assert (Path(tmp_dir) / "RELEASE-NOTES.md").exists() + assert (Path(tmp_dir) / "file_only_present_on_a_particular_tag.txt").exists() import json print(json.dumps(result.json, indent=2)) From 20e362fe9313a645663a087d465eb5c295eb883a Mon Sep 17 00:00:00 2001 From: Patryk Czajka Date: Mon, 4 Mar 2024 13:49:21 +0100 Subject: [PATCH 11/29] refactor tests --- tests_integration/__snapshots__/test_git.ambr | 11 +- tests_integration/test_git.py | 126 ++++++++++++------ 2 files changed, 98 insertions(+), 39 deletions(-) diff --git a/tests_integration/__snapshots__/test_git.ambr b/tests_integration/__snapshots__/test_git.ambr index b49d4ce029..2627b9b5cb 100644 --- a/tests_integration/__snapshots__/test_git.ambr +++ b/tests_integration/__snapshots__/test_git.ambr @@ -1,5 +1,14 @@ # serializer version: 1 -# name: test_flow[list-files-not-stage-error] +# name: test_copy + ''' + ╭─ Error ──────────────────────────────────────────────────────────────────────╮ + │ REPOSITORY_PATH should be a path to git repository stage with scope │ + │ provided. For example: @my_repo/branches/main │ + ╰──────────────────────────────────────────────────────────────────────────────╯ + + ''' +# --- +# name: test_list_files ''' ╭─ Error ──────────────────────────────────────────────────────────────────────╮ │ REPOSITORY_PATH should be a path to git repository stage with scope │ diff --git a/tests_integration/test_git.py b/tests_integration/test_git.py index 7b0657bff4..224a53c318 100644 --- a/tests_integration/test_git.py +++ b/tests_integration/test_git.py @@ -3,6 +3,9 @@ from pathlib import Path import tempfile +TAG_NAME = "a-particular-tag" +BRANCH_NAME = "a-branch-which-is-different-than-main" + @pytest.fixture def git_repository(runner, test_database): @@ -41,14 +44,11 @@ def git_repository(runner, test_database): @pytest.mark.integration -def test_flow(runner, git_repository, snapshot): - TAG_NAME = "a-particular-tag" - BRANCH_NAME = "a-branch-which-is-different-than-main" - +def test_object_commands(runner, git_repository): # object list result = runner.invoke_with_connection_json(["object", "list", "git-repository"]) assert result.exit_code == 0 - _assert_object_with_key_value(result.json, key="name", value=git_repository) + assert _filter_key(result.json, key="name") == [git_repository] # describe result = runner.invoke_with_connection_json( @@ -57,7 +57,16 @@ def test_flow(runner, git_repository, snapshot): assert result.exit_code == 0 assert result.json[0]["name"] == git_repository - # fetch + # drop + result = runner.invoke_with_connection_json( + ["object", "drop", "git-repository", git_repository] + ) + assert result.exit_code == 0 + assert result.json == [{"status": "SNOWCLI_DUMMY_REPO successfully dropped."}] + + +@pytest.mark.integration +def test_fetch(runner, git_repository): result = runner.invoke_with_connection_json(["git", "fetch", git_repository]) assert result.exit_code == 0 assert result.json == [ @@ -66,22 +75,48 @@ def test_flow(runner, git_repository, snapshot): } ] + +@pytest.mark.integration +def test_list_branches_and_tags(runner, git_repository): # list branches result = runner.invoke_with_connection_json( ["git", "list-branches", git_repository] ) assert result.exit_code == 0 - _assert_object_with_key_value(result.json, key="name", value="main") - _assert_object_with_key_value(result.json, key="name", value=BRANCH_NAME) + assert result.json == [ + { + "checkouts": "", + "commit_hash": "f1b8cf60445d9d4c9bee32501738df55d0b4312e", + "name": "a-branch-which-is-different-than-main", + "path": "/branches/a-branch-which-is-different-than-main", + }, + { + "checkouts": "", + "commit_hash": "599e77bdbf59e29451f1a909baa2734f96b6c801", + "name": "main", + "path": "/branches/main", + }, + ] # list tags result = runner.invoke_with_connection_json(["git", "list-tags", git_repository]) assert result.exit_code == 0 - _assert_object_with_key_value(result.json, key="name", value=TAG_NAME) + assert result.json == [ + { + "author": None, + "commit_hash": "8fb57b3f1cf69c84274e760e86b16eb9933b45d5", + "message": None, + "name": "a-particular-tag", + "path": "/tags/a-particular-tag", + }, + ] - # list files - error messages + +@pytest.mark.integration +def test_list_files(runner, git_repository, snapshot): + # error messages result = runner.invoke_with_connection(["git", "list-files", git_repository]) - assert result.output == snapshot(name="list-files-not-stage-error") + assert result.output == snapshot try: repository_path = f"@{git_repository}" @@ -111,22 +146,31 @@ def test_flow(runner, git_repository, snapshot): == "The specified tag 'tag_which_does_not_exist' cannot be found in the Git Repository." ) - # list-files - success + # list-files - branch - no '/' at the end repository_path = f"@{git_repository}/branches/{BRANCH_NAME}" result = runner.invoke_with_connection_json(["git", "list-files", repository_path]) - _assert_object_with_key_value( - result.json, - key="name", - value=f"{repository_path[1:]}/file_only_present_on_a_different_branch.txt", - ) - repository_path = f"@{git_repository}/tags/{TAG_NAME}" + assert result.exit_code == 0 + prefix = repository_path[1:].lower() + assert _filter_key(result.json, key="name") == [ + f"{prefix}/an_existing_directory/file_inside.txt", + f"{prefix}/file.txt", + f"{prefix}/file_only_present_on_a_different_branch.txt", + ] + + # list-files - tags - '/' at the end + repository_path = f"@{git_repository}/tags/{TAG_NAME}/" result = runner.invoke_with_connection_json(["git", "list-files", repository_path]) - _assert_object_with_key_value( - result.json, - key="name", - value=f"{repository_path[1:]}/file_only_present_on_a_particular_tag.txt", - ) + assert result.exit_code == 0 + prefix = repository_path[1:-1].lower() + assert _filter_key(result.json, key="name") == [ + f"{prefix}/an_existing_directory/file_inside.txt", + f"{prefix}/file.txt", + f"{prefix}/file_only_present_on_a_particular_tag.txt", + ] + +@pytest.mark.integration +def test_copy(runner, git_repository, snapshot): # create stage for testing copy STAGE_NAME = "a_perfect_stage_for_testing" result = runner.invoke_with_connection(["sql", "-q", f"create stage {STAGE_NAME}"]) @@ -136,7 +180,7 @@ def test_flow(runner, git_repository, snapshot): result = runner.invoke_with_connection( ["git", "copy", git_repository, f"@{STAGE_NAME}"] ) - assert result.output == snapshot(name="copy-not-stage-error") + assert result.output == snapshot try: repository_path = f"@{git_repository}" runner.invoke_with_connection( @@ -177,31 +221,37 @@ def test_flow(runner, git_repository, snapshot): ["git", "copy", repository_path, f"@{STAGE_NAME}"] ) assert result.exit_code == 0 - _assert_object_with_key_value( - result.json, key="file", value="file_only_present_on_a_different_branch.txt" - ) + assert result.json == [ + {"file": "an_existing_directory/file_inside.txt"}, + {"file": "file.txt"}, + {"file": "file_only_present_on_a_different_branch.txt"}, + ] + result = runner.invoke_with_connection_json(["object", "stage", "list", STAGE_NAME]) - _assert_object_with_key_value( - result.json, - key="name", - value=f"{STAGE_NAME}/file_only_present_on_a_different_branch.txt", - ) + assert result.exit_code == 0 + assert _filter_key(result.json, key="name") == [ + f"{STAGE_NAME}/an_existing_directory/file_inside.txt", + f"{STAGE_NAME}/file.txt", + f"{STAGE_NAME}/file_only_present_on_a_different_branch.txt", + ] + # copy to local file system - success with tempfile.TemporaryDirectory() as tmp_dir: - # copy to local file system - success repository_path = f"@{git_repository}/tags/{TAG_NAME}" result = runner.invoke_with_connection_json( ["git", "copy", repository_path, tmp_dir] ) assert result.exit_code == 0 assert (Path(tmp_dir) / "file_only_present_on_a_particular_tag.txt").exists() - import json - - print(json.dumps(result.json, indent=2)) + assert _filter_key(result.json, key="file") == [ + f"an_existing_directory/file_inside.txt", + f"file.txt", + f"file_only_present_on_a_particular_tag.txt", + ] -def _assert_object_with_key_value(objects, *, key, value): - assert any(object[key] == value for object in objects) +def _filter_key(objects, *, key): + return [o[key] for o in objects] def _integration_exists(runner, integration_name): From adbef0cff603b99f4c42435a8ac76eb73014aad6 Mon Sep 17 00:00:00 2001 From: Patryk Czajka Date: Mon, 4 Mar 2024 13:58:45 +0100 Subject: [PATCH 12/29] update unit tests --- tests/git/test_git_commands.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/git/test_git_commands.py b/tests/git/test_git_commands.py index be252284b8..4248b91241 100644 --- a/tests/git/test_git_commands.py +++ b/tests/git/test_git_commands.py @@ -68,10 +68,11 @@ def test_copy(mock_connector, runner, mock_ctx): local_path = Path("local/path") result = runner.invoke(["git", "copy", "@repo_name/branches/main", str(local_path)]) + # paths in generated SQL should end with '/' assert result.exit_code == 0, result.output assert ( ctx.get_query() - == f"get @repo_name/branches/main file://{local_path.resolve()}/ parallel=4" + == f"get @repo_name/branches/main/ file://{local_path.resolve()}/ parallel=4" ) ctx = mock_ctx() From 712b07b5e692624c34caabe07d64801615ce5f0f Mon Sep 17 00:00:00 2001 From: Patryk Czajka Date: Mon, 4 Mar 2024 15:43:25 +0100 Subject: [PATCH 13/29] remove snapshots for Windows run --- tests_integration/__snapshots__/test_git.ambr | 19 ------------------- tests_integration/test_git.py | 16 ++++++++++++---- 2 files changed, 12 insertions(+), 23 deletions(-) delete mode 100644 tests_integration/__snapshots__/test_git.ambr diff --git a/tests_integration/__snapshots__/test_git.ambr b/tests_integration/__snapshots__/test_git.ambr deleted file mode 100644 index 2627b9b5cb..0000000000 --- a/tests_integration/__snapshots__/test_git.ambr +++ /dev/null @@ -1,19 +0,0 @@ -# serializer version: 1 -# name: test_copy - ''' - ╭─ Error ──────────────────────────────────────────────────────────────────────╮ - │ REPOSITORY_PATH should be a path to git repository stage with scope │ - │ provided. For example: @my_repo/branches/main │ - ╰──────────────────────────────────────────────────────────────────────────────╯ - - ''' -# --- -# name: test_list_files - ''' - ╭─ Error ──────────────────────────────────────────────────────────────────────╮ - │ REPOSITORY_PATH should be a path to git repository stage with scope │ - │ provided. For example: @my_repo/branches/main │ - ╰──────────────────────────────────────────────────────────────────────────────╯ - - ''' -# --- diff --git a/tests_integration/test_git.py b/tests_integration/test_git.py index 224a53c318..d1d30d2117 100644 --- a/tests_integration/test_git.py +++ b/tests_integration/test_git.py @@ -113,10 +113,10 @@ def test_list_branches_and_tags(runner, git_repository): @pytest.mark.integration -def test_list_files(runner, git_repository, snapshot): +def test_list_files(runner, git_repository): # error messages result = runner.invoke_with_connection(["git", "list-files", git_repository]) - assert result.output == snapshot + _assert_error_message_in_output(result.output) try: repository_path = f"@{git_repository}" @@ -170,7 +170,7 @@ def test_list_files(runner, git_repository, snapshot): @pytest.mark.integration -def test_copy(runner, git_repository, snapshot): +def test_copy(runner, git_repository): # create stage for testing copy STAGE_NAME = "a_perfect_stage_for_testing" result = runner.invoke_with_connection(["sql", "-q", f"create stage {STAGE_NAME}"]) @@ -180,7 +180,7 @@ def test_copy(runner, git_repository, snapshot): result = runner.invoke_with_connection( ["git", "copy", git_repository, f"@{STAGE_NAME}"] ) - assert result.output == snapshot + _assert_error_message_in_output(result.output) try: repository_path = f"@{git_repository}" runner.invoke_with_connection( @@ -254,6 +254,14 @@ def _filter_key(objects, *, key): return [o[key] for o in objects] +def _assert_error_message_in_output(output): + assert "Error" in output + assert ( + "REPOSITORY_PATH should be a path to git repository stage with scope" in output + ) + assert "provided. For example: @my_repo/branches/main" in output + + def _integration_exists(runner, integration_name): result = runner.invoke_with_connection_json(["sql", "-q", "SHOW INTEGRATIONS"]) assert result.exit_code == 0 From 9c596593ac6b03cbcf7f496d7ddf0c29ce2ca569 Mon Sep 17 00:00:00 2001 From: Patryk Czajka Date: Tue, 5 Mar 2024 10:44:03 +0100 Subject: [PATCH 14/29] target help message: a directory; create target if not exists --- src/snowflake/cli/plugins/git/commands.py | 4 +++- tests/__snapshots__/test_help_messages.ambr | 2 +- tests/git/test_git_commands.py | 6 ++++-- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/snowflake/cli/plugins/git/commands.py b/src/snowflake/cli/plugins/git/commands.py index c4804fa6ee..7d55573c59 100644 --- a/src/snowflake/cli/plugins/git/commands.py +++ b/src/snowflake/cli/plugins/git/commands.py @@ -77,7 +77,7 @@ def fetch( def copy( repository_path: str = RepoPathArgument, destination_path: str = typer.Argument( - help="Target path for copy operation. Should be stage or local file path.", + help="Target path for copy operation. Should be stage or a path to a local directory.", ), parallel: int = typer.Option( 4, @@ -92,6 +92,8 @@ def copy( ) else: target = Path(destination_path).resolve() + if not target.exists(): + target.mkdir() cursor = GitManager().get( stage_name=repository_path, dest_path=target, parallel=parallel ) diff --git a/tests/__snapshots__/test_help_messages.ambr b/tests/__snapshots__/test_help_messages.ambr index 08346fcdcb..a6f907d43e 100644 --- a/tests/__snapshots__/test_help_messages.ambr +++ b/tests/__snapshots__/test_help_messages.ambr @@ -769,7 +769,7 @@ │ [default: None] │ │ [required] │ │ * destination_path TEXT Target path for copy operation. Should be │ - │ stage or local file path. │ + │ stage or a path to a local directory. │ │ [default: None] │ │ [required] │ ╰──────────────────────────────────────────────────────────────────────────────╯ diff --git a/tests/git/test_git_commands.py b/tests/git/test_git_commands.py index bb159007aa..9d63df038f 100644 --- a/tests/git/test_git_commands.py +++ b/tests/git/test_git_commands.py @@ -57,13 +57,15 @@ def test_fetch(mock_connector, runner, mock_ctx): @mock.patch("snowflake.connector.connect") -def test_copy(mock_connector, runner, mock_ctx): +def test_copy(mock_connector, runner, mock_ctx, temp_dir): ctx = mock_ctx() mock_connector.return_value = ctx - local_path = Path("local/path") + local_path = Path(temp_dir) / "local_dir" + assert not local_path.exists() result = runner.invoke(["git", "copy", "@repo_name/branches/main", str(local_path)]) assert result.exit_code == 0, result.output + assert local_path.exists() assert ( ctx.get_query() == f"get @repo_name/branches/main file://{local_path.resolve()}/ parallel=4" From d11c38ee314df568bd74b0488f8379765293d1e3 Mon Sep 17 00:00:00 2001 From: Patryk Czajka Date: Tue, 5 Mar 2024 12:45:30 +0100 Subject: [PATCH 15/29] Use SecurePath --- src/snowflake/cli/plugins/git/commands.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/snowflake/cli/plugins/git/commands.py b/src/snowflake/cli/plugins/git/commands.py index 7d55573c59..8f31abc216 100644 --- a/src/snowflake/cli/plugins/git/commands.py +++ b/src/snowflake/cli/plugins/git/commands.py @@ -5,6 +5,7 @@ from snowflake.cli.api.commands.flags import identifier_argument from snowflake.cli.api.commands.snow_typer import SnowTyper from snowflake.cli.api.output.types import CommandResult, QueryResult +from snowflake.cli.api.secure_path import SecurePath from snowflake.cli.api.utils.path_utils import is_stage_path from snowflake.cli.plugins.git.manager import GitManager @@ -91,7 +92,7 @@ def copy( repo_path=repository_path, destination_path=destination_path ) else: - target = Path(destination_path).resolve() + target = SecurePath(Path(destination_path).resolve()) if not target.exists(): target.mkdir() cursor = GitManager().get( From 0ef4dcf4d0a8cb053c3400e01650adfe03fe024c Mon Sep 17 00:00:00 2001 From: Patryk Czajka Date: Tue, 5 Mar 2024 13:21:29 +0100 Subject: [PATCH 16/29] fix path type --- src/snowflake/cli/plugins/git/commands.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/snowflake/cli/plugins/git/commands.py b/src/snowflake/cli/plugins/git/commands.py index 8f31abc216..0aab23b9f0 100644 --- a/src/snowflake/cli/plugins/git/commands.py +++ b/src/snowflake/cli/plugins/git/commands.py @@ -96,6 +96,6 @@ def copy( if not target.exists(): target.mkdir() cursor = GitManager().get( - stage_name=repository_path, dest_path=target, parallel=parallel + stage_name=repository_path, dest_path=target.path, parallel=parallel ) return QueryResult(cursor) From 5af7287d5b1c035687af0aa326e86e0f46fdc293 Mon Sep 17 00:00:00 2001 From: Patryk Czajka Date: Tue, 5 Mar 2024 16:28:27 +0100 Subject: [PATCH 17/29] review fixes --- src/snowflake/cli/plugins/git/commands.py | 4 ++-- src/snowflake/cli/plugins/git/manager.py | 11 ----------- src/snowflake/cli/plugins/object/stage/manager.py | 13 +++++++++++++ tests/git/test_git_commands.py | 5 ++++- 4 files changed, 19 insertions(+), 14 deletions(-) diff --git a/src/snowflake/cli/plugins/git/commands.py b/src/snowflake/cli/plugins/git/commands.py index 0aab23b9f0..935e9e696d 100644 --- a/src/snowflake/cli/plugins/git/commands.py +++ b/src/snowflake/cli/plugins/git/commands.py @@ -88,8 +88,8 @@ def copy( ): is_copy = is_stage_path(destination_path) if is_copy: - cursor = GitManager().copy( - repo_path=repository_path, destination_path=destination_path + cursor = GitManager().copy_files( + source_path=repository_path, destination_path=destination_path ) else: target = SecurePath(Path(destination_path).resolve()) diff --git a/src/snowflake/cli/plugins/git/manager.py b/src/snowflake/cli/plugins/git/manager.py index 05a4a02292..04e4816f4b 100644 --- a/src/snowflake/cli/plugins/git/manager.py +++ b/src/snowflake/cli/plugins/git/manager.py @@ -18,14 +18,3 @@ def show_files(self, repo_path: str) -> SnowflakeCursor: def fetch(self, repo_name: str) -> SnowflakeCursor: query = f"alter git repository {repo_name} fetch" return self._execute_query(query) - - def _get_standard_stage_directory_path(self, path): - if not path.endswith("/"): - path += "/" - return self.get_standard_stage_name(path) - - def copy(self, repo_path: str, destination_path: str) -> SnowflakeCursor: - source = self._get_standard_stage_directory_path(repo_path) - destination = self._get_standard_stage_directory_path(destination_path) - query = f"copy files into {destination} from {source}" - return self._execute_query(query) diff --git a/src/snowflake/cli/plugins/object/stage/manager.py b/src/snowflake/cli/plugins/object/stage/manager.py index 8baa291818..d282b29b0b 100644 --- a/src/snowflake/cli/plugins/object/stage/manager.py +++ b/src/snowflake/cli/plugins/object/stage/manager.py @@ -26,6 +26,12 @@ def get_standard_stage_name(name: str) -> str: return f"@{name}" + @staticmethod + def get_standard_stage_directory_path(path): + if not path.endswith("/"): + path += "/" + return StageManager.get_standard_stage_name(path) + @staticmethod def get_stage_name_from_path(path: str): """ @@ -90,6 +96,13 @@ def put( ) return cursor + def copy_files(self, source_path: str, destination_path: str) -> SnowflakeCursor: + source = self.get_standard_stage_directory_path(source_path) + destination = self.get_standard_stage_directory_path(destination_path) + log.info("Copying files from %s to %s", source, destination) + query = f"copy files into {destination} from {source}" + return self._execute_query(query) + def remove( self, stage_name: str, path: str, role: Optional[str] = None ) -> SnowflakeCursor: diff --git a/tests/git/test_git_commands.py b/tests/git/test_git_commands.py index 9d63df038f..4d987888be 100644 --- a/tests/git/test_git_commands.py +++ b/tests/git/test_git_commands.py @@ -57,7 +57,7 @@ def test_fetch(mock_connector, runner, mock_ctx): @mock.patch("snowflake.connector.connect") -def test_copy(mock_connector, runner, mock_ctx, temp_dir): +def test_copy_to_local_file_system(mock_connector, runner, mock_ctx, temp_dir): ctx = mock_ctx() mock_connector.return_value = ctx local_path = Path(temp_dir) / "local_dir" @@ -71,6 +71,9 @@ def test_copy(mock_connector, runner, mock_ctx, temp_dir): == f"get @repo_name/branches/main file://{local_path.resolve()}/ parallel=4" ) + +@mock.patch("snowflake.connector.connect") +def test_copy_to_remote_dir(mock_connector, runner, mock_ctx): ctx = mock_ctx() mock_connector.return_value = ctx result = runner.invoke( From 2f7eb04184a2a2fdc9b57ed15a5e91216596f8f6 Mon Sep 17 00:00:00 2001 From: Patryk Czajka Date: Tue, 5 Mar 2024 16:40:01 +0100 Subject: [PATCH 18/29] Refactor stage.get --- src/snowflake/cli/plugins/git/commands.py | 2 +- src/snowflake/cli/plugins/object/stage/commands.py | 2 +- src/snowflake/cli/plugins/object/stage/manager.py | 6 +++--- tests/object/stage/test_stage.py | 6 +++--- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/snowflake/cli/plugins/git/commands.py b/src/snowflake/cli/plugins/git/commands.py index 935e9e696d..2842426351 100644 --- a/src/snowflake/cli/plugins/git/commands.py +++ b/src/snowflake/cli/plugins/git/commands.py @@ -96,6 +96,6 @@ def copy( if not target.exists(): target.mkdir() cursor = GitManager().get( - stage_name=repository_path, dest_path=target.path, parallel=parallel + stage_path=repository_path, dest_path=target.path, parallel=parallel ) return QueryResult(cursor) diff --git a/src/snowflake/cli/plugins/object/stage/commands.py b/src/snowflake/cli/plugins/object/stage/commands.py index 7783a098c4..592009672e 100644 --- a/src/snowflake/cli/plugins/object/stage/commands.py +++ b/src/snowflake/cli/plugins/object/stage/commands.py @@ -69,7 +69,7 @@ def copy( if is_get: target = Path(destination_path).resolve() cursor = StageManager().get( - stage_name=source_path, dest_path=target, parallel=parallel + stage_path=source_path, dest_path=target, parallel=parallel ) else: source = Path(source_path).resolve() diff --git a/src/snowflake/cli/plugins/object/stage/manager.py b/src/snowflake/cli/plugins/object/stage/manager.py index d282b29b0b..9ca41c315b 100644 --- a/src/snowflake/cli/plugins/object/stage/manager.py +++ b/src/snowflake/cli/plugins/object/stage/manager.py @@ -64,12 +64,12 @@ def list_files(self, stage_name: str) -> SnowflakeCursor: return self._execute_query(f"ls {self.quote_stage_name(stage_name)}") def get( - self, stage_name: str, dest_path: Path, parallel: int = 4 + self, stage_path: str, dest_path: Path, parallel: int = 4 ) -> SnowflakeCursor: - stage_name = self.get_standard_stage_name(stage_name) + stage_path = self.get_standard_stage_directory_path(stage_path) dest_directory = f"{dest_path}/" return self._execute_query( - f"get {self.quote_stage_name(stage_name)} {self._to_uri(dest_directory)} parallel={parallel}" + f"get {self.quote_stage_name(stage_path)} {self._to_uri(dest_directory)} parallel={parallel}" ) def put( diff --git a/tests/object/stage/test_stage.py b/tests/object/stage/test_stage.py index dbf121162e..5c1036c7fa 100644 --- a/tests/object/stage/test_stage.py +++ b/tests/object/stage/test_stage.py @@ -34,7 +34,7 @@ def test_stage_copy_remote_to_local(mock_execute, runner, mock_cursor): ) assert result.exit_code == 0, result.output mock_execute.assert_called_once_with( - f"get @stageName file://{Path(tmp_dir).resolve()}/ parallel=4" + f"get @stageName/ file://{Path(tmp_dir).resolve()}/ parallel=4" ) @@ -47,7 +47,7 @@ def test_stage_copy_remote_to_local_quoted_stage(mock_execute, runner, mock_curs ) assert result.exit_code == 0, result.output mock_execute.assert_called_once_with( - f"get '@\"stage name\"' file://{Path(tmp_dir).resolve()}/ parallel=4" + f"get '@\"stage name\"/' file://{Path(tmp_dir).resolve()}/ parallel=4" ) @@ -80,7 +80,7 @@ def test_stage_copy_remote_to_local_quoted_uri( ["object", "stage", "copy", "-c", "empty", "@stageName", local_path] ) assert result.exit_code == 0, result.output - mock_execute.assert_called_once_with(f"get @stageName {file_uri} parallel=4") + mock_execute.assert_called_once_with(f"get @stageName/ {file_uri} parallel=4") @mock.patch(f"{STAGE_MANAGER}._execute_query") From c3de383b6a408cd6c2e5fa6dbde9892b39e833df Mon Sep 17 00:00:00 2001 From: Patryk Czajka Date: Tue, 5 Mar 2024 16:56:56 +0100 Subject: [PATCH 19/29] assert in callback --- src/snowflake/cli/plugins/git/commands.py | 15 ++++++++++++++- tests/git/test_git_commands.py | 23 ++++++++++++++++++++++- 2 files changed, 36 insertions(+), 2 deletions(-) diff --git a/src/snowflake/cli/plugins/git/commands.py b/src/snowflake/cli/plugins/git/commands.py index 2842426351..1a717c195a 100644 --- a/src/snowflake/cli/plugins/git/commands.py +++ b/src/snowflake/cli/plugins/git/commands.py @@ -2,6 +2,7 @@ from pathlib import Path import typer +from click import ClickException from snowflake.cli.api.commands.flags import identifier_argument from snowflake.cli.api.commands.snow_typer import SnowTyper from snowflake.cli.api.output.types import CommandResult, QueryResult @@ -16,9 +17,21 @@ ) log = logging.getLogger(__name__) + +def _repo_path_argument_callback(path): + if not is_stage_path(path): + raise ClickException( + f"REPOSITORY_PATH should be a path to git repository stage with scope provided." + " For example: @my_repo/branches/main" + ) + return path + + RepoNameArgument = identifier_argument(sf_object="git repository", example="my_repo") RepoPathArgument = typer.Argument( - help="Path to git repository stage with scope provided. For example: @my_repo/branches/main" + metavar="REPOSITORY_PATH", + help="Path to git repository stage with scope provided. For example: @my_repo/branches/main", + callback=_repo_path_argument_callback, ) diff --git a/tests/git/test_git_commands.py b/tests/git/test_git_commands.py index 4d987888be..58a6c9e1ec 100644 --- a/tests/git/test_git_commands.py +++ b/tests/git/test_git_commands.py @@ -46,6 +46,12 @@ def test_list_files(mock_connector, runner, mock_ctx): assert ctx.get_query() == "ls @repo_name/branches/main" +def test_list_files_not_a_stage_error(runner): + result = runner.invoke(["git", "list-files", "repo_name/branches/main"]) + assert result.exit_code == 1 + _assert_error_message(result.output) + + @mock.patch("snowflake.connector.connect") def test_fetch(mock_connector, runner, mock_ctx): ctx = mock_ctx() @@ -66,9 +72,10 @@ def test_copy_to_local_file_system(mock_connector, runner, mock_ctx, temp_dir): assert result.exit_code == 0, result.output assert local_path.exists() + # paths in generated SQL should end with '/' assert ( ctx.get_query() - == f"get @repo_name/branches/main file://{local_path.resolve()}/ parallel=4" + == f"get @repo_name/branches/main/ file://{local_path.resolve()}/ parallel=4" ) @@ -86,3 +93,17 @@ def test_copy_to_remote_dir(mock_connector, runner, mock_ctx): ctx.get_query() == "copy files into @stage_path/dir_in_stage/ from @repo_name/branches/main/" ) + + +def test_copy_not_a_stage_error(runner): + result = runner.invoke(["git", "copy", "repo_name", "@stage_path/dir_in_stage"]) + assert result.exit_code == 1 + _assert_error_message(result.output) + + +def _assert_error_message(output): + assert "Error" in output + assert ( + "REPOSITORY_PATH should be a path to git repository stage with scope" in output + ) + assert "provided. For example: @my_repo/branches/main" in output From cdf311d732417de7389ca6991dd1179b5331a380 Mon Sep 17 00:00:00 2001 From: Patryk Czajka Date: Tue, 5 Mar 2024 17:28:44 +0100 Subject: [PATCH 20/29] fix source error --- .../cli/plugins/object/stage/manager.py | 16 ++++++++-------- src/snowflake/cli/plugins/streamlit/manager.py | 2 +- tests/git/test_git_commands.py | 6 ++---- tests/object/stage/test_stage.py | 6 +++--- 4 files changed, 14 insertions(+), 16 deletions(-) diff --git a/src/snowflake/cli/plugins/object/stage/manager.py b/src/snowflake/cli/plugins/object/stage/manager.py index 9ca41c315b..ab5a36ab52 100644 --- a/src/snowflake/cli/plugins/object/stage/manager.py +++ b/src/snowflake/cli/plugins/object/stage/manager.py @@ -19,7 +19,7 @@ class StageManager(SqlExecutionMixin): @staticmethod - def get_standard_stage_name(name: str) -> str: + def get_standard_stage_prefix(name: str) -> str: # Handle embedded stages if name.startswith("snow://") or name.startswith("@"): return name @@ -30,7 +30,7 @@ def get_standard_stage_name(name: str) -> str: def get_standard_stage_directory_path(path): if not path.endswith("/"): path += "/" - return StageManager.get_standard_stage_name(path) + return StageManager.get_standard_stage_prefix(path) @staticmethod def get_stage_name_from_path(path: str): @@ -45,7 +45,7 @@ def quote_stage_name(name: str) -> str: if name.startswith("'") and name.endswith("'"): return name # already quoted - standard_name = StageManager.get_standard_stage_name(name) + standard_name = StageManager.get_standard_stage_prefix(name) if standard_name.startswith("@") and not re.fullmatch( r"@([\w./$])+", standard_name ): @@ -60,13 +60,13 @@ def _to_uri(self, local_path: str): return to_string_literal(uri) def list_files(self, stage_name: str) -> SnowflakeCursor: - stage_name = self.get_standard_stage_name(stage_name) + stage_name = self.get_standard_stage_prefix(stage_name) return self._execute_query(f"ls {self.quote_stage_name(stage_name)}") def get( self, stage_path: str, dest_path: Path, parallel: int = 4 ) -> SnowflakeCursor: - stage_path = self.get_standard_stage_directory_path(stage_path) + stage_path = self.get_standard_stage_prefix(stage_path) dest_directory = f"{dest_path}/" return self._execute_query( f"get {self.quote_stage_name(stage_path)} {self._to_uri(dest_directory)} parallel={parallel}" @@ -87,7 +87,7 @@ def put( and switch back to the original role for the next commands to run. """ with self.use_role(role) if role else nullcontext(): - stage_path = self.get_standard_stage_name(stage_path) + stage_path = self.get_standard_stage_prefix(stage_path) local_resolved_path = path_resolver(str(local_path)) log.info("Uploading %s to @%s", local_resolved_path, stage_path) cursor = self._execute_query( @@ -97,7 +97,7 @@ def put( return cursor def copy_files(self, source_path: str, destination_path: str) -> SnowflakeCursor: - source = self.get_standard_stage_directory_path(source_path) + source = self.get_standard_stage_prefix(source_path) destination = self.get_standard_stage_directory_path(destination_path) log.info("Copying files from %s to %s", source, destination) query = f"copy files into {destination} from {source}" @@ -113,7 +113,7 @@ def remove( and switch back to the original role for the next commands to run. """ with self.use_role(role) if role else nullcontext(): - stage_name = self.get_standard_stage_name(stage_name) + stage_name = self.get_standard_stage_prefix(stage_name) path = path if path.startswith("/") else "/" + path quoted_stage_name = self.quote_stage_name(f"{stage_name}{path}") return self._execute_query(f"remove {quoted_stage_name}") diff --git a/src/snowflake/cli/plugins/streamlit/manager.py b/src/snowflake/cli/plugins/streamlit/manager.py index 3beb317a39..8ae63e0385 100644 --- a/src/snowflake/cli/plugins/streamlit/manager.py +++ b/src/snowflake/cli/plugins/streamlit/manager.py @@ -153,7 +153,7 @@ def deploy( stage_manager.create(stage_name=stage_name) - root_location = stage_manager.get_standard_stage_name( + root_location = stage_manager.get_standard_stage_prefix( f"{stage_name}/{streamlit_name_for_root_location}" ) diff --git a/tests/git/test_git_commands.py b/tests/git/test_git_commands.py index 58a6c9e1ec..aae26350b2 100644 --- a/tests/git/test_git_commands.py +++ b/tests/git/test_git_commands.py @@ -72,10 +72,9 @@ def test_copy_to_local_file_system(mock_connector, runner, mock_ctx, temp_dir): assert result.exit_code == 0, result.output assert local_path.exists() - # paths in generated SQL should end with '/' assert ( ctx.get_query() - == f"get @repo_name/branches/main/ file://{local_path.resolve()}/ parallel=4" + == f"get @repo_name/branches/main file://{local_path.resolve()}/ parallel=4" ) @@ -87,11 +86,10 @@ def test_copy_to_remote_dir(mock_connector, runner, mock_ctx): ["git", "copy", "@repo_name/branches/main", "@stage_path/dir_in_stage"] ) - # paths in generated SQL should end with '/' assert result.exit_code == 0, result.output assert ( ctx.get_query() - == "copy files into @stage_path/dir_in_stage/ from @repo_name/branches/main/" + == "copy files into @stage_path/dir_in_stage/ from @repo_name/branches/main" ) diff --git a/tests/object/stage/test_stage.py b/tests/object/stage/test_stage.py index 5c1036c7fa..dbf121162e 100644 --- a/tests/object/stage/test_stage.py +++ b/tests/object/stage/test_stage.py @@ -34,7 +34,7 @@ def test_stage_copy_remote_to_local(mock_execute, runner, mock_cursor): ) assert result.exit_code == 0, result.output mock_execute.assert_called_once_with( - f"get @stageName/ file://{Path(tmp_dir).resolve()}/ parallel=4" + f"get @stageName file://{Path(tmp_dir).resolve()}/ parallel=4" ) @@ -47,7 +47,7 @@ def test_stage_copy_remote_to_local_quoted_stage(mock_execute, runner, mock_curs ) assert result.exit_code == 0, result.output mock_execute.assert_called_once_with( - f"get '@\"stage name\"/' file://{Path(tmp_dir).resolve()}/ parallel=4" + f"get '@\"stage name\"' file://{Path(tmp_dir).resolve()}/ parallel=4" ) @@ -80,7 +80,7 @@ def test_stage_copy_remote_to_local_quoted_uri( ["object", "stage", "copy", "-c", "empty", "@stageName", local_path] ) assert result.exit_code == 0, result.output - mock_execute.assert_called_once_with(f"get @stageName/ {file_uri} parallel=4") + mock_execute.assert_called_once_with(f"get @stageName {file_uri} parallel=4") @mock.patch(f"{STAGE_MANAGER}._execute_query") From 287511c3323aa8e0c069152224b7dc32214f1e3f Mon Sep 17 00:00:00 2001 From: Patryk Czajka Date: Tue, 5 Mar 2024 17:31:24 +0100 Subject: [PATCH 21/29] update help message --- src/snowflake/cli/plugins/git/commands.py | 2 +- tests/__snapshots__/test_help_messages.ambr | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/snowflake/cli/plugins/git/commands.py b/src/snowflake/cli/plugins/git/commands.py index 1a717c195a..1e750e5a1a 100644 --- a/src/snowflake/cli/plugins/git/commands.py +++ b/src/snowflake/cli/plugins/git/commands.py @@ -91,7 +91,7 @@ def fetch( def copy( repository_path: str = RepoPathArgument, destination_path: str = typer.Argument( - help="Target path for copy operation. Should be stage or a path to a local directory.", + help="Target path for copy operation. Should be a path to a directory on remote stage or local file system.", ), parallel: int = typer.Option( 4, diff --git a/tests/__snapshots__/test_help_messages.ambr b/tests/__snapshots__/test_help_messages.ambr index 0a5dd09eea..52251fe4ad 100644 --- a/tests/__snapshots__/test_help_messages.ambr +++ b/tests/__snapshots__/test_help_messages.ambr @@ -768,8 +768,9 @@ │ @my_repo/branches/main │ │ [default: None] │ │ [required] │ - │ * destination_path TEXT Target path for copy operation. Should be │ - │ stage or a path to a local directory. │ + │ * destination_path TEXT Target path for copy operation. Should be a │ + │ path to a directory on remote stage or │ + │ local file system. │ │ [default: None] │ │ [required] │ ╰──────────────────────────────────────────────────────────────────────────────╯ From 2317d2970538755a668947d77c422a55ea25b339 Mon Sep 17 00:00:00 2001 From: Patryk Czajka Date: Wed, 6 Mar 2024 13:07:35 +0100 Subject: [PATCH 22/29] Review fixes --- src/snowflake/cli/api/secure_path.py | 26 ++++++------ src/snowflake/cli/plugins/git/commands.py | 41 +++++++++++++------ .../cli/plugins/object/stage/manager.py | 9 ++++ tests/__snapshots__/test_help_messages.ambr | 13 ++++-- 4 files changed, 59 insertions(+), 30 deletions(-) diff --git a/src/snowflake/cli/api/secure_path.py b/src/snowflake/cli/api/secure_path.py index 11d8d5b6b7..50ab4effb2 100644 --- a/src/snowflake/cli/api/secure_path.py +++ b/src/snowflake/cli/api/secure_path.py @@ -52,8 +52,8 @@ def iterdir(self): For details, check pathlib.Path.iterdir() """ - self._assert_exists() - self._assert_is_directory() + self.assert_exists() + self.assert_is_directory() return (SecurePath(p) for p in self._path.iterdir()) def exists(self) -> bool: @@ -134,7 +134,7 @@ def open( # noqa: A003 self._assert_file_size_limit(read_file_limit_mb) if self.exists(): - self._assert_is_file() + self.assert_is_file() else: self.touch() # makes sure permissions of freshly-created file are strict @@ -175,7 +175,7 @@ def copy( and files within the destination tree will be overwritten by corresponding files from the src tree. """ - self._assert_exists() + self.assert_exists() destination = Path(destination) if destination.exists(): @@ -226,10 +226,10 @@ def unlink(self, missing_ok=False): """ if not self.exists(): if not missing_ok: - self._assert_exists() + self.assert_exists() return - self._assert_is_file() + self.assert_is_file() log.info("Removing file %s", self._path) self._path.unlink() @@ -244,10 +244,10 @@ def rmdir(self, recursive=False, missing_ok=False): """ if not self.exists(): if not missing_ok: - self._assert_exists() + self.assert_exists() return - self._assert_is_directory() + self.assert_is_directory() if not recursive and any(self._path.iterdir()): raise DirectoryIsNotEmptyError(self._path.resolve()) @@ -271,20 +271,20 @@ def temporary_directory(cls): log.info("Removing temporary directory %s", tmpdir) def _assert_exists_and_is_file(self) -> None: - self._assert_exists() - self._assert_is_file() + self.assert_exists() + self.assert_is_file() - def _assert_exists(self) -> None: + def assert_exists(self) -> None: if not self.exists(): raise FileNotFoundError( errno.ENOENT, os.strerror(errno.ENOENT), self._path.resolve() ) - def _assert_is_file(self) -> None: + def assert_is_file(self) -> None: if not self._path.is_file(): _raise_is_a_directory_error(self._path.resolve()) - def _assert_is_directory(self) -> None: + def assert_is_directory(self) -> None: if not self._path.is_dir(): _raise_not_a_directory_error(self._path.resolve()) diff --git a/src/snowflake/cli/plugins/git/commands.py b/src/snowflake/cli/plugins/git/commands.py index 1e750e5a1a..13be906546 100644 --- a/src/snowflake/cli/plugins/git/commands.py +++ b/src/snowflake/cli/plugins/git/commands.py @@ -6,7 +6,6 @@ from snowflake.cli.api.commands.flags import identifier_argument from snowflake.cli.api.commands.snow_typer import SnowTyper from snowflake.cli.api.output.types import CommandResult, QueryResult -from snowflake.cli.api.secure_path import SecurePath from snowflake.cli.api.utils.path_utils import is_stage_path from snowflake.cli.plugins.git.manager import GitManager @@ -21,8 +20,8 @@ def _repo_path_argument_callback(path): if not is_stage_path(path): raise ClickException( - f"REPOSITORY_PATH should be a path to git repository stage with scope provided." - " For example: @my_repo/branches/main" + "REPOSITORY_PATH should be a path to git repository stage with scope provided." + " For example: @my_repo/branches/main/" ) return path @@ -30,62 +29,73 @@ def _repo_path_argument_callback(path): RepoNameArgument = identifier_argument(sf_object="git repository", example="my_repo") RepoPathArgument = typer.Argument( metavar="REPOSITORY_PATH", - help="Path to git repository stage with scope provided. For example: @my_repo/branches/main", + help=( + "Path to git repository stage with scope provided." + " Path to the repository root must end with '/'." + " For example: @my_repo/branches/main/" + ), callback=_repo_path_argument_callback, ) @app.command( "list-branches", - help="List all branches in the repository.", requires_connection=True, ) def list_branches( repository_name: str = RepoNameArgument, **options, ) -> CommandResult: + """ + List all branches in the repository. + """ return QueryResult(GitManager().show_branches(repo_name=repository_name)) @app.command( "list-tags", - help="List all tags in the repository.", requires_connection=True, ) def list_tags( repository_name: str = RepoNameArgument, **options, ) -> CommandResult: + """ + List all tags in the repository. + """ return QueryResult(GitManager().show_tags(repo_name=repository_name)) @app.command( "list-files", - help="List files from given state of git repository.", requires_connection=True, ) def list_files( repository_path: str = RepoPathArgument, **options, ) -> CommandResult: + """ + List files from given state of git repository. + """ return QueryResult(GitManager().show_files(repo_path=repository_path)) @app.command( "fetch", - help="Fetch changes from origin to snowflake repository.", requires_connection=True, ) def fetch( repository_name: str = RepoNameArgument, **options, ) -> CommandResult: + """ + Fetch changes from origin to snowflake repository. + """ return QueryResult(GitManager().fetch(repo_name=repository_name)) @app.command( "copy", - help="Copies all files from given state of repository to local directory or stage.", requires_connection=True, ) def copy( @@ -99,16 +109,21 @@ def copy( ), **options, ): + """ + Copies all files from given state of repository to local directory or stage. + + If the source path ends with '/', the command copies contents of specified directory. + Otherwise, it creates a new directory or file in the destination directory. + """ is_copy = is_stage_path(destination_path) if is_copy: cursor = GitManager().copy_files( source_path=repository_path, destination_path=destination_path ) else: - target = SecurePath(Path(destination_path).resolve()) - if not target.exists(): - target.mkdir() cursor = GitManager().get( - stage_path=repository_path, dest_path=target.path, parallel=parallel + stage_path=repository_path, + dest_path=Path(destination_path).resolve(), + parallel=parallel, ) return QueryResult(cursor) diff --git a/src/snowflake/cli/plugins/object/stage/manager.py b/src/snowflake/cli/plugins/object/stage/manager.py index ab5a36ab52..d369824493 100644 --- a/src/snowflake/cli/plugins/object/stage/manager.py +++ b/src/snowflake/cli/plugins/object/stage/manager.py @@ -7,6 +7,7 @@ from typing import Optional, Union from snowflake.cli.api.project.util import to_string_literal +from snowflake.cli.api.secure_path import SecurePath from snowflake.cli.api.sql_execution import SqlExecutionMixin from snowflake.cli.api.utils.path_utils import path_resolver from snowflake.connector.cursor import SnowflakeCursor @@ -63,10 +64,18 @@ def list_files(self, stage_name: str) -> SnowflakeCursor: stage_name = self.get_standard_stage_prefix(stage_name) return self._execute_query(f"ls {self.quote_stage_name(stage_name)}") + @staticmethod + def _assure_is_existing_directory(path: Path) -> None: + spath = SecurePath(path) + if not spath.exists(): + spath.mkdir() + spath.assert_is_directory() + def get( self, stage_path: str, dest_path: Path, parallel: int = 4 ) -> SnowflakeCursor: stage_path = self.get_standard_stage_prefix(stage_path) + self._assure_is_existing_directory(dest_path) dest_directory = f"{dest_path}/" return self._execute_query( f"get {self.quote_stage_name(stage_path)} {self._to_uri(dest_directory)} parallel={parallel}" diff --git a/tests/__snapshots__/test_help_messages.ambr b/tests/__snapshots__/test_help_messages.ambr index 52251fe4ad..871132e3e2 100644 --- a/tests/__snapshots__/test_help_messages.ambr +++ b/tests/__snapshots__/test_help_messages.ambr @@ -761,11 +761,15 @@ Usage: default git copy [OPTIONS] REPOSITORY_PATH DESTINATION_PATH Copies all files from given state of repository to local directory or stage. + If the source path ends with '/', the command copies contents of specified + directory. Otherwise, it creates a new directory or file in the destination + directory. ╭─ Arguments ──────────────────────────────────────────────────────────────────╮ │ * repository_path TEXT Path to git repository stage with scope │ - │ provided. For example: │ - │ @my_repo/branches/main │ + │ provided. Path to the repository root must │ + │ end with '/'. For example: │ + │ @my_repo/branches/main/ │ │ [default: None] │ │ [required] │ │ * destination_path TEXT Target path for copy operation. Should be a │ @@ -971,8 +975,9 @@ ╭─ Arguments ──────────────────────────────────────────────────────────────────╮ │ * repository_path TEXT Path to git repository stage with scope │ - │ provided. For example: │ - │ @my_repo/branches/main │ + │ provided. Path to the repository root must │ + │ end with '/'. For example: │ + │ @my_repo/branches/main/ │ │ [default: None] │ │ [required] │ ╰──────────────────────────────────────────────────────────────────────────────╯ From 7265e17085f07f07e4379932b11575fdb4916d78 Mon Sep 17 00:00:00 2001 From: Patryk Czajka Date: Wed, 6 Mar 2024 15:02:27 +0100 Subject: [PATCH 23/29] rename test assert --- tests/git/test_git_commands.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/git/test_git_commands.py b/tests/git/test_git_commands.py index aae26350b2..ee9fa199b6 100644 --- a/tests/git/test_git_commands.py +++ b/tests/git/test_git_commands.py @@ -49,7 +49,7 @@ def test_list_files(mock_connector, runner, mock_ctx): def test_list_files_not_a_stage_error(runner): result = runner.invoke(["git", "list-files", "repo_name/branches/main"]) assert result.exit_code == 1 - _assert_error_message(result.output) + _assert_invalid_repo_path_error_message(result.output) @mock.patch("snowflake.connector.connect") @@ -96,10 +96,10 @@ def test_copy_to_remote_dir(mock_connector, runner, mock_ctx): def test_copy_not_a_stage_error(runner): result = runner.invoke(["git", "copy", "repo_name", "@stage_path/dir_in_stage"]) assert result.exit_code == 1 - _assert_error_message(result.output) + _assert_invalid_repo_path_error_message(result.output) -def _assert_error_message(output): +def _assert_invalid_repo_path_error_message(output): assert "Error" in output assert ( "REPOSITORY_PATH should be a path to git repository stage with scope" in output From c2172f8cd1d36fd217b05ecab65e91cdce712d2c Mon Sep 17 00:00:00 2001 From: Patryk Czajka Date: Wed, 6 Mar 2024 15:05:02 +0100 Subject: [PATCH 24/29] small fix --- tests/git/test_git_commands.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/git/test_git_commands.py b/tests/git/test_git_commands.py index ee9fa199b6..fab949042b 100644 --- a/tests/git/test_git_commands.py +++ b/tests/git/test_git_commands.py @@ -104,4 +104,4 @@ def _assert_invalid_repo_path_error_message(output): assert ( "REPOSITORY_PATH should be a path to git repository stage with scope" in output ) - assert "provided. For example: @my_repo/branches/main" in output + assert "provided. For example: @my_repo/branches/main/" in output From a1a79ef770f61b94b612bd9609a5e59059b20289 Mon Sep 17 00:00:00 2001 From: Patryk Czajka Date: Wed, 6 Mar 2024 15:13:08 +0100 Subject: [PATCH 25/29] fix merge issues --- src/snowflake/cli/plugins/git/commands.py | 8 -------- tests/git/test_git_commands.py | 4 +--- 2 files changed, 1 insertion(+), 11 deletions(-) diff --git a/src/snowflake/cli/plugins/git/commands.py b/src/snowflake/cli/plugins/git/commands.py index 8ff25cf894..13be906546 100644 --- a/src/snowflake/cli/plugins/git/commands.py +++ b/src/snowflake/cli/plugins/git/commands.py @@ -127,11 +127,3 @@ def copy( parallel=parallel, ) return QueryResult(cursor) - - -def _assert_repository_path_is_stage(argument_name, path): - if not is_stage_path(path): - raise ClickException( - f"{argument_name} should be a path to git repository stage with scope provided." - " For example: @my_repo/branches/main" - ) diff --git a/tests/git/test_git_commands.py b/tests/git/test_git_commands.py index 7f5d9ae635..fab949042b 100644 --- a/tests/git/test_git_commands.py +++ b/tests/git/test_git_commands.py @@ -70,12 +70,11 @@ def test_copy_to_local_file_system(mock_connector, runner, mock_ctx, temp_dir): assert not local_path.exists() result = runner.invoke(["git", "copy", "@repo_name/branches/main", str(local_path)]) - # paths in generated SQL should end with '/' assert result.exit_code == 0, result.output assert local_path.exists() assert ( ctx.get_query() - == f"get @repo_name/branches/main/ file://{local_path.resolve()}/ parallel=4" + == f"get @repo_name/branches/main file://{local_path.resolve()}/ parallel=4" ) @@ -105,5 +104,4 @@ def _assert_invalid_repo_path_error_message(output): assert ( "REPOSITORY_PATH should be a path to git repository stage with scope" in output ) - assert "provided. For example: @my_repo/branches/main/" in output From dd6c1ae9f8c5f3bce86689805d69546e3e267baf Mon Sep 17 00:00:00 2001 From: Patryk Czajka Date: Wed, 6 Mar 2024 16:23:26 +0100 Subject: [PATCH 26/29] change repo to snowcli; refactor copy tests --- tests_integration/test_git.py | 169 ++++++++++++++++++---------------- 1 file changed, 92 insertions(+), 77 deletions(-) diff --git a/tests_integration/test_git.py b/tests_integration/test_git.py index d1d30d2117..ad1fe42de5 100644 --- a/tests_integration/test_git.py +++ b/tests_integration/test_git.py @@ -3,14 +3,13 @@ from pathlib import Path import tempfile -TAG_NAME = "a-particular-tag" -BRANCH_NAME = "a-branch-which-is-different-than-main" +FILE_IN_REPO = "RELEASE-NOTES.md" @pytest.fixture def git_repository(runner, test_database): - repo_name = "SNOWCLI_DUMMY_REPO" - integration_name = "SNOWCLI_DUMMY_REPO_API_INTEGRATION" + repo_name = "SNOWCLI_TESTING_REPO" + integration_name = "SNOW_GIT_TESTING_API_INTEGRATION" if not _integration_exists(runner, integration_name=integration_name): result = runner.invoke_with_connection( @@ -20,7 +19,7 @@ def git_repository(runner, test_database): f""" CREATE API INTEGRATION {integration_name} API_PROVIDER = git_https_api - API_ALLOWED_PREFIXES = ('https://github.com/sfc-gh-pczajka') + API_ALLOWED_PREFIXES = ('https://github.com/snowflakedb/') ALLOWED_AUTHENTICATION_SECRETS = () ENABLED = true """, @@ -35,7 +34,7 @@ def git_repository(runner, test_database): f""" CREATE GIT REPOSITORY {repo_name} API_INTEGRATION = {integration_name} - ORIGIN = 'https://github.com/sfc-gh-pczajka/dummy-repo-for-snowcli-testing.git' + ORIGIN = 'https://github.com/snowflakedb/snowflake-cli.git' """, ] ) @@ -48,7 +47,7 @@ def test_object_commands(runner, git_repository): # object list result = runner.invoke_with_connection_json(["object", "list", "git-repository"]) assert result.exit_code == 0 - assert _filter_key(result.json, key="name") == [git_repository] + assert git_repository in _filter_key(result.json, key="name") # describe result = runner.invoke_with_connection_json( @@ -62,7 +61,7 @@ def test_object_commands(runner, git_repository): ["object", "drop", "git-repository", git_repository] ) assert result.exit_code == 0 - assert result.json == [{"status": "SNOWCLI_DUMMY_REPO successfully dropped."}] + assert result.json == [{"status": f"{git_repository} successfully dropped."}] @pytest.mark.integration @@ -83,33 +82,12 @@ def test_list_branches_and_tags(runner, git_repository): ["git", "list-branches", git_repository] ) assert result.exit_code == 0 - assert result.json == [ - { - "checkouts": "", - "commit_hash": "f1b8cf60445d9d4c9bee32501738df55d0b4312e", - "name": "a-branch-which-is-different-than-main", - "path": "/branches/a-branch-which-is-different-than-main", - }, - { - "checkouts": "", - "commit_hash": "599e77bdbf59e29451f1a909baa2734f96b6c801", - "name": "main", - "path": "/branches/main", - }, - ] + assert "main" in _filter_key(result.json, key="name") # list tags result = runner.invoke_with_connection_json(["git", "list-tags", git_repository]) assert result.exit_code == 0 - assert result.json == [ - { - "author": None, - "commit_hash": "8fb57b3f1cf69c84274e760e86b16eb9933b45d5", - "message": None, - "name": "a-particular-tag", - "path": "/tags/a-particular-tag", - }, - ] + assert "v2.0.0" in _filter_key(result.json, key="name") @pytest.mark.integration @@ -146,35 +124,18 @@ def test_list_files(runner, git_repository): == "The specified tag 'tag_which_does_not_exist' cannot be found in the Git Repository." ) - # list-files - branch - no '/' at the end - repository_path = f"@{git_repository}/branches/{BRANCH_NAME}" + repository_path = f"@{git_repository}/tags/v2.1.0-rc1/" result = runner.invoke_with_connection_json(["git", "list-files", repository_path]) assert result.exit_code == 0 - prefix = repository_path[1:].lower() - assert _filter_key(result.json, key="name") == [ - f"{prefix}/an_existing_directory/file_inside.txt", - f"{prefix}/file.txt", - f"{prefix}/file_only_present_on_a_different_branch.txt", - ] - - # list-files - tags - '/' at the end - repository_path = f"@{git_repository}/tags/{TAG_NAME}/" - result = runner.invoke_with_connection_json(["git", "list-files", repository_path]) - assert result.exit_code == 0 - prefix = repository_path[1:-1].lower() - assert _filter_key(result.json, key="name") == [ - f"{prefix}/an_existing_directory/file_inside.txt", - f"{prefix}/file.txt", - f"{prefix}/file_only_present_on_a_particular_tag.txt", - ] + assert f"{repository_path[1:].lower()}{FILE_IN_REPO}" in _filter_key( + result.json, key="name" + ) +@pytest.mark.skip(reason="in progress") @pytest.mark.integration -def test_copy(runner, git_repository): - # create stage for testing copy - STAGE_NAME = "a_perfect_stage_for_testing" - result = runner.invoke_with_connection(["sql", "-q", f"create stage {STAGE_NAME}"]) - assert result.exit_code == 0 +def test_copy_error_messages(runner, git_repository): + STAGE_NAME = "omitted" # copy - test error messages result = runner.invoke_with_connection( @@ -215,39 +176,93 @@ def test_copy(runner, git_repository): == "The specified tag 'tag_which_does_not_exist' cannot be found in the Git Repository." ) - # copy to stage - success - repository_path = f"@{git_repository}/branches/{BRANCH_NAME}" + +@pytest.mark.integration +def test_copy_to_stage(runner, git_repository): + REPO_PATH_PREFIX = f"@{git_repository}/tags/v2.1.0-rc0" + SUBDIR = "tests_integration/config" + SUBDIR_ON_STAGE = "config" + FILE_IN_SUBDIR = "connection_configs.toml" + STAGE_NAME = "a_perfect_stage_for_testing" + + def _assert_file_on_stage(file_path): + result = runner.invoke_with_connection_json( + ["object", "stage", "list", STAGE_NAME] + ) + assert result.exit_code == 0 + print([f["name"] for f in result.json]) + assert f"{STAGE_NAME.lower()}/{file_path}" in [f["name"] for f in result.json] + + # create stage for testing copy + result = runner.invoke_with_connection(["object", "stage", "create", STAGE_NAME]) + assert result.exit_code == 0 + + # copy directory - whole directory + repository_path = f"{REPO_PATH_PREFIX}/{SUBDIR}" result = runner.invoke_with_connection_json( ["git", "copy", repository_path, f"@{STAGE_NAME}"] ) assert result.exit_code == 0 - assert result.json == [ - {"file": "an_existing_directory/file_inside.txt"}, - {"file": "file.txt"}, - {"file": "file_only_present_on_a_different_branch.txt"}, - ] + _assert_file_on_stage(f"{SUBDIR_ON_STAGE}/{FILE_IN_SUBDIR}") # whole dir is copied - result = runner.invoke_with_connection_json(["object", "stage", "list", STAGE_NAME]) + # copy directory - copy contents + repository_path = f"{REPO_PATH_PREFIX}/{SUBDIR}/" + result = runner.invoke_with_connection_json( + ["git", "copy", repository_path, f"@{STAGE_NAME}"] + ) assert result.exit_code == 0 - assert _filter_key(result.json, key="name") == [ - f"{STAGE_NAME}/an_existing_directory/file_inside.txt", - f"{STAGE_NAME}/file.txt", - f"{STAGE_NAME}/file_only_present_on_a_different_branch.txt", - ] + _assert_file_on_stage(FILE_IN_SUBDIR) # contents are copied + + # copy single file + repository_path = f"{REPO_PATH_PREFIX}/{FILE_IN_REPO}" + result = runner.invoke_with_connection_json( + ["git", "copy", repository_path, f"@{STAGE_NAME}"] + ) + assert result.exit_code == 0 + _assert_file_on_stage(FILE_IN_REPO) + + # copy file into directory + repository_path = f"{REPO_PATH_PREFIX}/{FILE_IN_REPO}" + result = runner.invoke_with_connection_json( + ["git", "copy", repository_path, f"@{STAGE_NAME}/a_dir/"] + ) + assert result.exit_code == 0 + _assert_file_on_stage(f"a_dir/{FILE_IN_REPO}") + # error with no '/' at the end should be fixed by snowcli + repository_path = f"{REPO_PATH_PREFIX}/{FILE_IN_REPO}" + result = runner.invoke_with_connection_json( + ["git", "copy", repository_path, f"@{STAGE_NAME}/another_dir"] + ) + assert result.exit_code == 0 + _assert_file_on_stage(f"another_dir/{FILE_IN_REPO}") + - # copy to local file system - success +@pytest.mark.integration +def test_copy_to_local_file_system(runner, git_repository): + # TODO: change subdir to dedicated one after merging this to main + REPO_PATH_PREFIX = f"@{git_repository}/tags/v2.1.0-rc0" + SUBDIR = "tests_integration/config" + FILE_IN_SUBDIR = "connection_configs.toml" with tempfile.TemporaryDirectory() as tmp_dir: - repository_path = f"@{git_repository}/tags/{TAG_NAME}" + LOCAL_DIR = Path(tmp_dir) / "a_dir" + assert not LOCAL_DIR.exists() + + # copy directory - GET only copy contents + repository_path = f"{REPO_PATH_PREFIX}/{SUBDIR}" result = runner.invoke_with_connection_json( - ["git", "copy", repository_path, tmp_dir] + ["git", "copy", repository_path, str(LOCAL_DIR)] ) assert result.exit_code == 0 - assert (Path(tmp_dir) / "file_only_present_on_a_particular_tag.txt").exists() - assert _filter_key(result.json, key="file") == [ - f"an_existing_directory/file_inside.txt", - f"file.txt", - f"file_only_present_on_a_particular_tag.txt", - ] + assert LOCAL_DIR.exists() # create directory if not exists + assert (LOCAL_DIR / FILE_IN_SUBDIR).exists() # contents are copied + + # copy single file + repository_path = f"{REPO_PATH_PREFIX}/{FILE_IN_REPO}" + result = runner.invoke_with_connection_json( + ["git", "copy", repository_path, str(LOCAL_DIR)] + ) + assert result.exit_code == 0 + assert (LOCAL_DIR / FILE_IN_REPO).exists() def _filter_key(objects, *, key): From d51002b523c397506cdcb577215a18cef6be3800 Mon Sep 17 00:00:00 2001 From: Patryk Czajka Date: Thu, 7 Mar 2024 10:15:35 +0100 Subject: [PATCH 27/29] review fixes --- tests_integration/test_git.py | 121 ++++++++++------------------------ 1 file changed, 36 insertions(+), 85 deletions(-) diff --git a/tests_integration/test_git.py b/tests_integration/test_git.py index ad1fe42de5..6bf1493380 100644 --- a/tests_integration/test_git.py +++ b/tests_integration/test_git.py @@ -7,7 +7,7 @@ @pytest.fixture -def git_repository(runner, test_database): +def sf_git_repository(runner, test_database): repo_name = "SNOWCLI_TESTING_REPO" integration_name = "SNOW_GIT_TESTING_API_INTEGRATION" @@ -43,62 +43,63 @@ def git_repository(runner, test_database): @pytest.mark.integration -def test_object_commands(runner, git_repository): +def test_object_commands(runner, sf_git_repository): # object list result = runner.invoke_with_connection_json(["object", "list", "git-repository"]) assert result.exit_code == 0 - assert git_repository in _filter_key(result.json, key="name") + assert sf_git_repository in _filter_key(result.json, key="name") # describe result = runner.invoke_with_connection_json( - ["object", "describe", "git-repository", git_repository] + ["object", "describe", "git-repository", sf_git_repository] ) assert result.exit_code == 0 - assert result.json[0]["name"] == git_repository + assert result.json[0]["name"] == sf_git_repository # drop result = runner.invoke_with_connection_json( - ["object", "drop", "git-repository", git_repository] + ["object", "drop", "git-repository", sf_git_repository] ) assert result.exit_code == 0 - assert result.json == [{"status": f"{git_repository} successfully dropped."}] + assert result.json == [{"status": f"{sf_git_repository} successfully dropped."}] @pytest.mark.integration -def test_fetch(runner, git_repository): - result = runner.invoke_with_connection_json(["git", "fetch", git_repository]) +def test_fetch(runner, sf_git_repository): + result = runner.invoke_with_connection_json(["git", "fetch", sf_git_repository]) assert result.exit_code == 0 assert result.json == [ { - "status": f"Git Repository {git_repository} is up to date. No change was fetched." + "status": f"Git Repository {sf_git_repository} is up to date. No change was fetched." } ] @pytest.mark.integration -def test_list_branches_and_tags(runner, git_repository): +def test_list_branches_and_tags(runner, sf_git_repository): # list branches result = runner.invoke_with_connection_json( - ["git", "list-branches", git_repository] + ["git", "list-branches", sf_git_repository] ) assert result.exit_code == 0 assert "main" in _filter_key(result.json, key="name") # list tags - result = runner.invoke_with_connection_json(["git", "list-tags", git_repository]) + result = runner.invoke_with_connection_json(["git", "list-tags", sf_git_repository]) assert result.exit_code == 0 assert "v2.0.0" in _filter_key(result.json, key="name") @pytest.mark.integration -def test_list_files(runner, git_repository): - # error messages - result = runner.invoke_with_connection(["git", "list-files", git_repository]) +def test_list_files(runner, sf_git_repository): + # error messages are passed to the user + result = runner.invoke_with_connection(["git", "list-files", sf_git_repository]) _assert_error_message_in_output(result.output) try: - repository_path = f"@{git_repository}" + repository_path = f"@{sf_git_repository}" runner.invoke_with_connection(["git", "list-files", repository_path]) + assert False, "Expected exception" except ProgrammingError as err: assert ( err.raw_msg @@ -106,25 +107,7 @@ def test_list_files(runner, git_repository): "a tag name, or a valid commit hash. Commit hashes are between 6 and 40 characters long." ) - try: - repository_path = f"@{git_repository}/branches/branch_which_does_not_exist" - runner.invoke_with_connection(["git", "list-files", repository_path]) - except ProgrammingError as err: - assert ( - err.raw_msg - == "The specified branch 'branch_which_does_not_exist' cannot be found in the Git Repository." - ) - - try: - repository_path = f"@{git_repository}/tags/tag_which_does_not_exist" - runner.invoke_with_connection(["git", "list-files", repository_path]) - except ProgrammingError as err: - assert ( - err.raw_msg - == "The specified tag 'tag_which_does_not_exist' cannot be found in the Git Repository." - ) - - repository_path = f"@{git_repository}/tags/v2.1.0-rc1/" + repository_path = f"@{sf_git_repository}/tags/v2.1.0-rc1/" result = runner.invoke_with_connection_json(["git", "list-files", repository_path]) assert result.exit_code == 0 assert f"{repository_path[1:].lower()}{FILE_IN_REPO}" in _filter_key( @@ -132,54 +115,9 @@ def test_list_files(runner, git_repository): ) -@pytest.mark.skip(reason="in progress") -@pytest.mark.integration -def test_copy_error_messages(runner, git_repository): - STAGE_NAME = "omitted" - - # copy - test error messages - result = runner.invoke_with_connection( - ["git", "copy", git_repository, f"@{STAGE_NAME}"] - ) - _assert_error_message_in_output(result.output) - try: - repository_path = f"@{git_repository}" - runner.invoke_with_connection( - ["git", "copy", repository_path, f"@{STAGE_NAME}"] - ) - except ProgrammingError as err: - assert ( - err.raw_msg - == "Files paths in git repositories must specify a scope. For example, a branch name, " - "a tag name, or a valid commit hash. Commit hashes are between 6 and 40 characters long." - ) - - try: - repository_path = f"@{git_repository}/branches/branch_which_does_not_exist" - runner.invoke_with_connection( - ["git", "copy", repository_path, f"@{STAGE_NAME}"] - ) - except ProgrammingError as err: - assert ( - err.raw_msg - == "The specified branch 'branch_which_does_not_exist' cannot be found in the Git Repository." - ) - - try: - repository_path = f"@{git_repository}/tags/tag_which_does_not_exist" - runner.invoke_with_connection( - ["git", "copy", repository_path, f"@{STAGE_NAME}"] - ) - except ProgrammingError as err: - assert ( - err.raw_msg - == "The specified tag 'tag_which_does_not_exist' cannot be found in the Git Repository." - ) - - @pytest.mark.integration -def test_copy_to_stage(runner, git_repository): - REPO_PATH_PREFIX = f"@{git_repository}/tags/v2.1.0-rc0" +def test_copy_to_stage(runner, sf_git_repository): + REPO_PATH_PREFIX = f"@{sf_git_repository}/tags/v2.1.0-rc0" SUBDIR = "tests_integration/config" SUBDIR_ON_STAGE = "config" FILE_IN_SUBDIR = "connection_configs.toml" @@ -238,9 +176,9 @@ def _assert_file_on_stage(file_path): @pytest.mark.integration -def test_copy_to_local_file_system(runner, git_repository): +def test_copy_to_local_file_system(runner, sf_git_repository): # TODO: change subdir to dedicated one after merging this to main - REPO_PATH_PREFIX = f"@{git_repository}/tags/v2.1.0-rc0" + REPO_PATH_PREFIX = f"@{sf_git_repository}/tags/v2.1.0-rc0" SUBDIR = "tests_integration/config" FILE_IN_SUBDIR = "connection_configs.toml" with tempfile.TemporaryDirectory() as tmp_dir: @@ -264,6 +202,19 @@ def test_copy_to_local_file_system(runner, git_repository): assert result.exit_code == 0 assert (LOCAL_DIR / FILE_IN_REPO).exists() + # error messages are passed to the user + try: + repository_path = f"@{sf_git_repository}/tags/no-such-tag/" + runner.invoke_with_connection( + ["git", "copy", repository_path, str(LOCAL_DIR)] + ) + assert False, "Expected exception" + except ProgrammingError as err: + assert ( + err.raw_msg + == "The specified tag 'no-such-tag' cannot be found in the Git Repository." + ) + def _filter_key(objects, *, key): return [o[key] for o in objects] From c8fc64cf38e8f2a09219f61c6c1a7633af636cb6 Mon Sep 17 00:00:00 2001 From: Patryk Czajka Date: Thu, 7 Mar 2024 15:23:18 +0100 Subject: [PATCH 28/29] rename function --- tests_integration/test_git.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests_integration/test_git.py b/tests_integration/test_git.py index 6bf1493380..e3faac0dba 100644 --- a/tests_integration/test_git.py +++ b/tests_integration/test_git.py @@ -94,7 +94,7 @@ def test_list_branches_and_tags(runner, sf_git_repository): def test_list_files(runner, sf_git_repository): # error messages are passed to the user result = runner.invoke_with_connection(["git", "list-files", sf_git_repository]) - _assert_error_message_in_output(result.output) + _assert_invalid_repo_path_error_message(result.output) try: repository_path = f"@{sf_git_repository}" @@ -220,7 +220,7 @@ def _filter_key(objects, *, key): return [o[key] for o in objects] -def _assert_error_message_in_output(output): +def _assert_invalid_repo_path_error_message(output): assert "Error" in output assert ( "REPOSITORY_PATH should be a path to git repository stage with scope" in output From adc7450ee24c32a30547ad565c676e21c54bacf4 Mon Sep 17 00:00:00 2001 From: Patryk Czajka Date: Thu, 7 Mar 2024 15:23:56 +0100 Subject: [PATCH 29/29] remove debug --- tests_integration/test_git.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests_integration/test_git.py b/tests_integration/test_git.py index e3faac0dba..5ae9e1a96a 100644 --- a/tests_integration/test_git.py +++ b/tests_integration/test_git.py @@ -128,7 +128,6 @@ def _assert_file_on_stage(file_path): ["object", "stage", "list", STAGE_NAME] ) assert result.exit_code == 0 - print([f["name"] for f in result.json]) assert f"{STAGE_NAME.lower()}/{file_path}" in [f["name"] for f in result.json] # create stage for testing copy