From cb61b027c71e19549702135c30c4781a1ff15163 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Job?= Date: Wed, 23 Oct 2024 18:03:38 +0200 Subject: [PATCH] [SNOW-1758029] Removed necessity of requirements.txt for git execute (#1776) --- RELEASE-NOTES.md | 2 + src/snowflake/cli/_plugins/stage/manager.py | 10 ++-- tests/stage/test_stage.py | 27 +++++++--- tests_integration/__snapshots__/test_git.ambr | 9 ++++ .../__snapshots__/test_stage.ambr | 9 ++++ .../script_template.py | 13 +++++ tests_integration/test_git.py | 41 +++++++++++++++ tests_integration/test_stage.py | 51 +++++++++++++++++++ 8 files changed, 149 insertions(+), 13 deletions(-) create mode 100644 tests_integration/test_data/projects/stage_execute_without_requirements/script_template.py diff --git a/RELEASE-NOTES.md b/RELEASE-NOTES.md index d456e20edb..9c4dc0cb4b 100644 --- a/RELEASE-NOTES.md +++ b/RELEASE-NOTES.md @@ -31,6 +31,8 @@ * `snow spcs image-repository list-images` now displays image tag and digest. * Fix `snow stage list-files` for paths with directories. * `snow --info` callback returns information about `SNOWFLAKE_HOME` variable. +* Removed requirement of existence of any `requirements.txt` file for Python code execution via `snow git execute` command. + Before the fix the file (even empty) was required to make the execution working. # v3.0.2 diff --git a/src/snowflake/cli/_plugins/stage/manager.py b/src/snowflake/cli/_plugins/stage/manager.py index 7e6b351c6e..625e161ef2 100644 --- a/src/snowflake/cli/_plugins/stage/manager.py +++ b/src/snowflake/cli/_plugins/stage/manager.py @@ -391,6 +391,8 @@ def execute( stage_path = self.build_path(stage_path_str) all_files_list = self._get_files_list_from_stage(stage_path.root_path()) + if not all_files_list: + raise ClickException(f"No files found on stage '{stage_path}'") all_files_with_stage_name_prefix = [ stage_path_parts.get_directory(file) for file in all_files_list @@ -480,10 +482,6 @@ def _get_files_list_from_stage( self, stage_path: StagePath, pattern: str | None = None ) -> List[str]: files_list_result = self.list_files(stage_path, pattern=pattern).fetchall() - - if not files_list_result: - raise ClickException(f"No files found on stage '{stage_path}'") - return [f["name"] for f in files_list_result] def _filter_files_list( @@ -631,7 +629,7 @@ def _bootstrap_snowpark_execution_environment(self, stage_path: StagePath): requirements = self._check_for_requirements_file(stage_path) self.snowpark_session.add_packages(*requirements) - @sproc(is_permanent=False) + @sproc(is_permanent=False, session=self.snowpark_session) def _python_execution_procedure( _: Session, file_path: str, variables: Dict | None = None ) -> None: @@ -668,7 +666,7 @@ def _execute_python( from snowflake.snowpark.exceptions import SnowparkSQLException try: - self._python_exe_procedure(self.get_standard_stage_prefix(file_stage_path), variables) # type: ignore + self._python_exe_procedure(self.get_standard_stage_prefix(file_stage_path), variables, session=self.snowpark_session) # type: ignore return StageManager._success_result(file=original_file) except SnowparkSQLException as e: StageManager._handle_execution_exception(on_error=on_error, exception=e) diff --git a/tests/stage/test_stage.py b/tests/stage/test_stage.py index 86944fd07a..ac10333699 100644 --- a/tests/stage/test_stage.py +++ b/tests/stage/test_stage.py @@ -867,8 +867,11 @@ def test_execute_from_user_stage( @mock.patch(f"{STAGE_MANAGER}._execute_query") @mock.patch(f"{STAGE_MANAGER}._bootstrap_snowpark_execution_environment") +@mock.patch(f"{STAGE_MANAGER}.snowpark_session") @skip_python_3_12 -def test_execute_with_variables(mock_bootstrap, mock_execute, mock_cursor, runner): +def test_execute_with_variables( + mock_snowpark_session, mock_bootstrap, mock_execute, mock_cursor, runner +): mock_execute.return_value = mock_cursor( [{"name": "exe/s1.sql"}, {"name": "exe/s2.py"}], [] ) @@ -907,6 +910,7 @@ def test_execute_with_variables(mock_bootstrap, mock_execute, mock_cursor, runne "key4": "NULL", "key5": "var=value", }, + session=mock_snowpark_session, ) @@ -1000,8 +1004,11 @@ def test_execute_no_files_for_stage_path( @mock.patch(f"{STAGE_MANAGER}._execute_query") @mock.patch(f"{STAGE_MANAGER}._bootstrap_snowpark_execution_environment") +@mock.patch(f"{STAGE_MANAGER}.snowpark_session") @skip_python_3_12 -def test_execute_stop_on_error(mock_bootstrap, mock_execute, mock_cursor, runner): +def test_execute_stop_on_error( + mock_snowpark_session, mock_bootstrap, mock_execute, mock_cursor, runner +): error_message = "Error" mock_execute.side_effect = [ mock_cursor( @@ -1027,17 +1034,23 @@ def test_execute_stop_on_error(mock_bootstrap, mock_execute, mock_cursor, runner mock.call(f"execute immediate from @exe/s2.sql"), ] assert mock_bootstrap.return_value.mock_calls == [ - mock.call("@exe/p1.py", {}), - mock.call("@exe/p2.py", {}), + mock.call("@exe/p1.py", {}, session=mock_snowpark_session), + mock.call("@exe/p2.py", {}, session=mock_snowpark_session), ] assert error_message in result.output @mock.patch(f"{STAGE_MANAGER}._execute_query") @mock.patch(f"{STAGE_MANAGER}._bootstrap_snowpark_execution_environment") +@mock.patch(f"{STAGE_MANAGER}.snowpark_session") @skip_python_3_12 def test_execute_continue_on_error( - mock_bootstrap, mock_execute, mock_cursor, runner, os_agnostic_snapshot + mock_snowpark_session, + mock_bootstrap, + mock_execute, + mock_cursor, + runner, + os_agnostic_snapshot, ): from snowflake.snowpark.exceptions import SnowparkSQLException @@ -1071,8 +1084,8 @@ def test_execute_continue_on_error( ] assert mock_bootstrap.return_value.mock_calls == [ - mock.call("@exe/p1.py", {}), - mock.call("@exe/p2.py", {}), + mock.call("@exe/p1.py", {}, session=mock_snowpark_session), + mock.call("@exe/p2.py", {}, session=mock_snowpark_session), ] diff --git a/tests_integration/__snapshots__/test_git.ambr b/tests_integration/__snapshots__/test_git.ambr index 0fd7fc4689..7845a2c219 100644 --- a/tests_integration/__snapshots__/test_git.ambr +++ b/tests_integration/__snapshots__/test_git.ambr @@ -17,6 +17,15 @@ }), ]) # --- +# name: test_git_execute_python_without_requirements + list([ + dict({ + 'Error': None, + 'File': '@snowcli_testing_repo/branches/main/tests_integration/test_data/projects/stage_execute_without_requirements/script_template.py', + 'Status': 'SUCCESS', + }), + ]) +# --- # name: test_execute_with_name_in_pascal_case list([ dict({ diff --git a/tests_integration/__snapshots__/test_stage.ambr b/tests_integration/__snapshots__/test_stage.ambr index a3f98c619d..93975a7886 100644 --- a/tests_integration/__snapshots__/test_stage.ambr +++ b/tests_integration/__snapshots__/test_stage.ambr @@ -91,6 +91,15 @@ }), ]) # --- +# name: test_stage_execute_python_without_requirements + list([ + dict({ + 'Error': None, + 'File': '@test_stage_execute_without_requirements/script_template.py', + 'Status': 'SUCCESS', + }), + ]) +# --- # name: test_user_stage_execute list([ dict({ diff --git a/tests_integration/test_data/projects/stage_execute_without_requirements/script_template.py b/tests_integration/test_data/projects/stage_execute_without_requirements/script_template.py new file mode 100644 index 0000000000..65f2427b6e --- /dev/null +++ b/tests_integration/test_data/projects/stage_execute_without_requirements/script_template.py @@ -0,0 +1,13 @@ +import os +from snowflake.core import Root +from snowflake.core.database import DatabaseResource +from snowflake.core.schema import Schema +from snowflake.snowpark.session import Session + +session = Session.builder.getOrCreate() +database: DatabaseResource = Root(session).databases[os.environ["test_database_name"]] + +assert database.name.upper() == os.environ["test_database_name"].upper() + +# Make a side effect that we can check in tests +database.schemas.create(Schema(name=os.environ["TEST_ID"])) diff --git a/tests_integration/test_git.py b/tests_integration/test_git.py index ce7868c753..af557fe014 100644 --- a/tests_integration/test_git.py +++ b/tests_integration/test_git.py @@ -11,12 +11,16 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. +import sys +import time import pytest from snowflake.connector.errors import ProgrammingError from pathlib import Path import tempfile +from tests_integration.test_utils import contains_row_with + FILE_IN_REPO = "RELEASE-NOTES.md" @@ -302,6 +306,43 @@ def test_execute_python(runner, test_database, sf_git_repository, snapshot): assert result.json == snapshot +@pytest.mark.integration +@pytest.mark.skipif( + sys.version_info >= (3, 12), reason="Snowpark is not supported in Python >= 3.12" +) +@pytest.mark.skip( + "Requires merging changes to the main branch" +) # TODO: remove after merging to the main branch +def test_git_execute_python_without_requirements( + snowflake_session, + runner, + test_database, + test_root_path, + snapshot, + sf_git_repository, +): + test_id = f"FOO{time.time_ns()}" + result = runner.invoke_with_connection_json( + [ + "git", + "execute", + f"@{sf_git_repository.lower()}/branches/main/tests_integration/test_data/projects/stage_execute_without_requirements", + "-D", + f"test_database_name={test_database}", + "-D", + f"TEST_ID={test_id}", + ] + ) + assert result.exit_code == 0 + assert result.json == snapshot + + # Assert side effect created by executed script + *_, schemas = snowflake_session.execute_string( + f"show schemas like '{test_id}' in database {test_database};" + ) + assert len(list(schemas)) == 1 + + @pytest.mark.integration def test_execute_fqn_repo(runner, test_database, sf_git_repository): result_fqn = runner.invoke_with_connection_json( diff --git a/tests_integration/test_stage.py b/tests_integration/test_stage.py index 2e22fece43..b04c89c42d 100644 --- a/tests_integration/test_stage.py +++ b/tests_integration/test_stage.py @@ -384,6 +384,57 @@ def test_stage_execute_python( assert len(list(schemas)) == 1 +@pytest.mark.integration +@pytest.mark.skipif( + sys.version_info >= (3, 12), reason="Snowpark is not supported in Python >= 3.12" +) +def test_stage_execute_python_without_requirements( + snowflake_session, runner, test_database, test_root_path, snapshot +): + project_path = ( + test_root_path / "test_data/projects/stage_execute_without_requirements" + ) + stage_name = "test_stage_execute_without_requirements" + + result = runner.invoke_with_connection_json(["stage", "create", stage_name]) + assert contains_row_with( + result.json, + {"status": f"Stage area {stage_name.upper()} successfully created."}, + ) + + result = runner.invoke_with_connection_json( + [ + "stage", + "copy", + str(Path(project_path) / "script_template.py"), + f"@{stage_name}", + ] + ) + assert result.exit_code == 0, result.output + assert contains_row_with(result.json, {"status": "UPLOADED"}) + + test_id = f"FOO{time.time_ns()}" + result = runner.invoke_with_connection_json( + [ + "stage", + "execute", + f"{stage_name}/", + "-D", + f"test_database_name={test_database}", + "-D", + f"TEST_ID={test_id}", + ] + ) + assert result.exit_code == 0 + assert result.json == snapshot + + # Assert side effect created by executed script + *_, schemas = snowflake_session.execute_string( + f"show schemas like '{test_id}' in database {test_database};" + ) + assert len(list(schemas)) == 1 + + @pytest.mark.integration def test_stage_diff(runner, snowflake_session, test_database, tmp_path, snapshot): stage_name = "test_stage"