From a6f2fb01eed3a0fd6b660f9a25964b745ef231d3 Mon Sep 17 00:00:00 2001 From: Bhumika Goel Date: Wed, 12 Jun 2024 23:13:24 -0500 Subject: [PATCH] rebased onto main with bundle changes, addressed PR feedback, test passes when using real config.toml connection locally, pushing to test on Github now fix code quality failure, loosen restriction on checking stderr when CLI's config.toml has insufficient permissions --- .../codegen/snowpark/callback_source.py.jinja | 1 - .../codegen/snowpark/python_processor.py | 8 +- tests_e2e/__snapshots__/test_nativeapp.ambr | 153 ++++++++++-------- tests_e2e/test_data/nativeapp.txt | 1 - .../nativeapp/python/cli_gen/errors/e1.py | 14 ++ .../nativeapp/python/cli_gen/ignored/i1.py | 14 ++ .../nativeapp/python/user_gen/echo.py | 15 ++ .../test_data/nativeapp/resources/unused.py | 13 ++ .../test_data/nativeapp/root_files/README.md | 16 ++ .../root_files/setup_scripts/setup.sql | 16 ++ .../setup_scripts/user_gen_procs.sql | 16 ++ .../root_files/setup_scripts/user_gen_udf.sql | 16 ++ tests_e2e/test_nativeapp.py | 53 ++++-- 13 files changed, 248 insertions(+), 88 deletions(-) delete mode 100644 tests_e2e/test_data/nativeapp.txt diff --git a/src/snowflake/cli/plugins/nativeapp/codegen/snowpark/callback_source.py.jinja b/src/snowflake/cli/plugins/nativeapp/codegen/snowpark/callback_source.py.jinja index 8b53c769c..9bc6bad41 100644 --- a/src/snowflake/cli/plugins/nativeapp/codegen/snowpark/callback_source.py.jinja +++ b/src/snowflake/cli/plugins/nativeapp/codegen/snowpark/callback_source.py.jinja @@ -102,7 +102,6 @@ def __snowflake_internal_create_extension_fn_registration_callback(): def __snowflake_internal_extension_fn_to_json(extension_fn): if not (isinstance(extension_fn.func, Callable) or isinstance(extension_fn.func, Tuple)): - # Unsupported case: extension function is a tuple return if extension_fn.anonymous: diff --git a/src/snowflake/cli/plugins/nativeapp/codegen/snowpark/python_processor.py b/src/snowflake/cli/plugins/nativeapp/codegen/snowpark/python_processor.py index 90f9045a0..f8bf17dc9 100644 --- a/src/snowflake/cli/plugins/nativeapp/codegen/snowpark/python_processor.py +++ b/src/snowflake/cli/plugins/nativeapp/codegen/snowpark/python_processor.py @@ -140,7 +140,7 @@ def _execute_in_sandbox( if completed_process.returncode != 0: cc.warning( - f"Could not fetch Snowpark objects from {py_file} due to the following Snowpark-internal error:\n {completed_process.stderr}" + f"Could not fetch Snowpark objects from {py_file} due to the following error:\n {completed_process.stderr}" ) cc.warning("Continuing execution for the rest of the python files.") return None @@ -211,6 +211,7 @@ def process( py_file=py_file, ) collected_sql_files.append(sql_file) + insert_newline = False for extension_fn in extension_fns: create_stmt = generate_create_sql_ddl_statement(extension_fn) if create_stmt is None: @@ -223,8 +224,11 @@ def process( collected_output.append(grant_statements) with open(sql_file, "a") as file: + if insert_newline: + file.write("\n") + insert_newline = True file.write( - f"\n-- Generated by the Snowflake CLI from {relative_py_file}\n" + f"-- Generated by the Snowflake CLI from {relative_py_file}\n" ) file.write(f"-- DO NOT EDIT\n") file.write(create_stmt) diff --git a/tests_e2e/__snapshots__/test_nativeapp.ambr b/tests_e2e/__snapshots__/test_nativeapp.ambr index 8325af1e8..37fab0dbb 100644 --- a/tests_e2e/__snapshots__/test_nativeapp.ambr +++ b/tests_e2e/__snapshots__/test_nativeapp.ambr @@ -1,91 +1,102 @@ # serializer version: 1 # name: test_full_lifecycle_with_codegen - ''' - call codegen_nativeapp_af3d785601654fa0a9a105883b626866_app.ext_code_schema.py_echo_proc('test') - +-----------------+ - | PY_ECHO_PROC | - |-----------------| - | echo_proc: test | - +-----------------+ - - ''' + dict_values(['echo_proc: test']) # --- # name: test_full_lifecycle_with_codegen.1 - ''' - select codegen_nativeapp_af3d785601654fa0a9a105883b626866_app.ext_code_schema.py_echo_fn('test') - +------------------------------------------------------------------------------+ - | CODEGEN_NATIVEAPP_AF3D785601654FA0A9A105883B626866_APP.EXT_CODE_SCHEMA.PY_EC | - | HO_FN('TEST') | - |------------------------------------------------------------------------------| - | echo_fn: test | - +------------------------------------------------------------------------------+ - - ''' + dict_values(['echo_fn: test']) +# --- +# name: test_full_lifecycle_with_codegen.10 + dict_values([10]) # --- # name: test_full_lifecycle_with_codegen.2 - ''' - select codegen_nativeapp_af3d785601654fa0a9a105883b626866_app.ext_code_schema.echo_fn_2('test') - +------------------------------------------------------------------------------+ - | CODEGEN_NATIVEAPP_AF3D785601654FA0A9A105883B626866_APP.EXT_CODE_SCHEMA.ECHO_ | - | FN_2('TEST') | - |------------------------------------------------------------------------------| - | echo_fn: test | - +------------------------------------------------------------------------------+ - - ''' + dict_values(['echo_fn: infile: test']) # --- # name: test_full_lifecycle_with_codegen.3 - ''' - select codegen_nativeapp_af3d785601654fa0a9a105883b626866_app.ext_code_schema.echo_fn_4('test') - +------------------------------------------------------------------------------+ - | CODEGEN_NATIVEAPP_AF3D785601654FA0A9A105883B626866_APP.EXT_CODE_SCHEMA.ECHO_ | - | FN_4('TEST') | - |------------------------------------------------------------------------------| - | echo_fn: test | - +------------------------------------------------------------------------------+ - - ''' + dict_values(['echo_fn: test']) # --- # name: test_full_lifecycle_with_codegen.4 - ''' - call codegen_nativeapp_af3d785601654fa0a9a105883b626866_app.ext_code_schema.add_sp(1, 2) - +--------+ - | ADD_SP | - |--------| - | 3 | - +--------+ - - ''' + dict_values(['echo_fn: test']) # --- # name: test_full_lifecycle_with_codegen.5 + dict_values([3]) +# --- +# name: test_full_lifecycle_with_codegen.6 + dict_values([10]) +# --- +# name: test_full_lifecycle_with_codegen.7 ''' - select codegen_nativeapp_af3d785601654fa0a9a105883b626866_app.ext_code_schema.sum_int_dec(10) - +------------------------------------------------------------------------------+ - | CODEGEN_NATIVEAPP_AF3D785601654FA0A9A105883B626866_APP.EXT_CODE_SCHEMA.SUM_I | - | NT_DEC(10) | - |------------------------------------------------------------------------------| - | 10 | - +------------------------------------------------------------------------------+ + [ + { + "NUMBER": 1 + }, + { + "NUMBER": -1 + }, + { + "NUMBER": 1 + }, + { + "NUMBER": -1 + }, + { + "NUMBER": 1 + }, + { + "NUMBER": -1 + }, + { + "NUMBER": 1 + }, + { + "NUMBER": -1 + }, + { + "NUMBER": 1 + }, + { + "NUMBER": -1 + } + ] ''' # --- -# name: test_full_lifecycle_with_codegen.6 +# name: test_full_lifecycle_with_codegen.8 + dict_values(['echo_fn: infile: test']) +# --- +# name: test_full_lifecycle_with_codegen.9 ''' - select * from TABLE(codegen_nativeapp_af3d785601654fa0a9a105883b626866_app.ext_code_schema.alt_int(10)) - +--------+ - | NUMBER | - |--------| - | 1 | - | -1 | - | 1 | - | -1 | - | 1 | - | -1 | - | 1 | - | -1 | - | 1 | - | -1 | - +--------+ + [ + { + "NUMBER": 1 + }, + { + "NUMBER": -1 + }, + { + "NUMBER": 1 + }, + { + "NUMBER": -1 + }, + { + "NUMBER": 1 + }, + { + "NUMBER": -1 + }, + { + "NUMBER": 1 + }, + { + "NUMBER": -1 + }, + { + "NUMBER": 1 + }, + { + "NUMBER": -1 + } + ] ''' # --- diff --git a/tests_e2e/test_data/nativeapp.txt b/tests_e2e/test_data/nativeapp.txt deleted file mode 100644 index fc11bbc93..000000000 --- a/tests_e2e/test_data/nativeapp.txt +++ /dev/null @@ -1 +0,0 @@ -# Dummy File with Fast Follow diff --git a/tests_e2e/test_data/nativeapp/python/cli_gen/errors/e1.py b/tests_e2e/test_data/nativeapp/python/cli_gen/errors/e1.py index a2ad2b1a4..283f9aa1c 100644 --- a/tests_e2e/test_data/nativeapp/python/cli_gen/errors/e1.py +++ b/tests_e2e/test_data/nativeapp/python/cli_gen/errors/e1.py @@ -1,3 +1,17 @@ +# Copyright (c) 2024 Snowflake Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# 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. + from snowflake.snowpark.functions import udf diff --git a/tests_e2e/test_data/nativeapp/python/cli_gen/ignored/i1.py b/tests_e2e/test_data/nativeapp/python/cli_gen/ignored/i1.py index 53e958423..2c87a9bb7 100644 --- a/tests_e2e/test_data/nativeapp/python/cli_gen/ignored/i1.py +++ b/tests_e2e/test_data/nativeapp/python/cli_gen/ignored/i1.py @@ -1,3 +1,17 @@ +# Copyright (c) 2024 Snowflake Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# 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. + from snowflake.snowpark.functions import udf # Should be ignored at callback, will process file but not generate any sql diff --git a/tests_e2e/test_data/nativeapp/python/user_gen/echo.py b/tests_e2e/test_data/nativeapp/python/user_gen/echo.py index c9b10d2be..563070bb0 100644 --- a/tests_e2e/test_data/nativeapp/python/user_gen/echo.py +++ b/tests_e2e/test_data/nativeapp/python/user_gen/echo.py @@ -1,3 +1,18 @@ +# Copyright (c) 2024 Snowflake Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# 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. + + def echo_fn(data): return "echo_fn: " + data diff --git a/tests_e2e/test_data/nativeapp/resources/unused.py b/tests_e2e/test_data/nativeapp/resources/unused.py index e69de29bb..ada0a4e13 100644 --- a/tests_e2e/test_data/nativeapp/resources/unused.py +++ b/tests_e2e/test_data/nativeapp/resources/unused.py @@ -0,0 +1,13 @@ +# Copyright (c) 2024 Snowflake Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# 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. diff --git a/tests_e2e/test_data/nativeapp/root_files/README.md b/tests_e2e/test_data/nativeapp/root_files/README.md index d5cb3fde5..6d474a135 100644 --- a/tests_e2e/test_data/nativeapp/root_files/README.md +++ b/tests_e2e/test_data/nativeapp/root_files/README.md @@ -1 +1,17 @@ + + DUMMY README FILE diff --git a/tests_e2e/test_data/nativeapp/root_files/setup_scripts/setup.sql b/tests_e2e/test_data/nativeapp/root_files/setup_scripts/setup.sql index 17a7bc5cd..6cf51daf4 100644 --- a/tests_e2e/test_data/nativeapp/root_files/setup_scripts/setup.sql +++ b/tests_e2e/test_data/nativeapp/root_files/setup_scripts/setup.sql @@ -1,3 +1,19 @@ +/* + Copyright (c) 2024 Snowflake Inc. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + 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. +*/ + create or replace application role app_instance_role; create or alter versioned schema ext_code_schema; diff --git a/tests_e2e/test_data/nativeapp/root_files/setup_scripts/user_gen_procs.sql b/tests_e2e/test_data/nativeapp/root_files/setup_scripts/user_gen_procs.sql index 3e376651a..d4212d293 100644 --- a/tests_e2e/test_data/nativeapp/root_files/setup_scripts/user_gen_procs.sql +++ b/tests_e2e/test_data/nativeapp/root_files/setup_scripts/user_gen_procs.sql @@ -1,3 +1,19 @@ +/* + Copyright (c) 2024 Snowflake Inc. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + 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. +*/ + -- user wraps a method from a python file into a procedure create or replace procedure ext_code_schema.py_echo_proc(STR string) RETURNS STRING diff --git a/tests_e2e/test_data/nativeapp/root_files/setup_scripts/user_gen_udf.sql b/tests_e2e/test_data/nativeapp/root_files/setup_scripts/user_gen_udf.sql index 723dbdbb5..fa466f3b1 100644 --- a/tests_e2e/test_data/nativeapp/root_files/setup_scripts/user_gen_udf.sql +++ b/tests_e2e/test_data/nativeapp/root_files/setup_scripts/user_gen_udf.sql @@ -1,3 +1,19 @@ +/* + Copyright (c) 2024 Snowflake Inc. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + 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. +*/ + -- user wraps a method from a python file into a function create or replace function ext_code_schema.py_echo_fn(STR string) RETURNS STRING diff --git a/tests_e2e/test_nativeapp.py b/tests_e2e/test_nativeapp.py index a5d070180..9e664bb55 100644 --- a/tests_e2e/test_nativeapp.py +++ b/tests_e2e/test_nativeapp.py @@ -12,6 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +import json import subprocess import uuid from pathlib import Path @@ -19,8 +20,6 @@ import pytest -from tests.nativeapp.utils import assert_dir_snapshot - def subprocess_check_output_with(sql_stmt: str, config_path: Path, snowcli) -> str: return subprocess.check_output( @@ -31,6 +30,8 @@ def subprocess_check_output_with(sql_stmt: str, config_path: Path, snowcli) -> s "sql", "-q", sql_stmt, + "--format", + "JSON", "-c", "integration", ], @@ -38,6 +39,34 @@ def subprocess_check_output_with(sql_stmt: str, config_path: Path, snowcli) -> s ) +def assert_snapshot_match_with_query_result(output: str, snapshot) -> bool: + """ + This function is required to parse the result value in the output independently from the uuid-based object created per test run. + E.g. + With the default format: + ''' + select codegen_nativeapp_af3d785601654fa0a9a105883b626866_app.ext_code_schema.py_echo_fn('test') + +------------------------------------------------------------------------------+ + | CODEGEN_NATIVEAPP_AF3D785601654FA0A9A105883B626866_APP.EXT_CODE_SCHEMA.PY_EC | + | HO_FN('TEST') | + |------------------------------------------------------------------------------| + | echo_fn: test | + +------------------------------------------------------------------------------+ + ''' + If the --format is JSON, then the output is like such: + ''' + [ + { + "CODEGEN_NATIVEAPP_E4B7C94AE8F74AF39D387CD44C4854E3_APP.EXT_CODE_SCHEMA.SUM_INT_DEC(10)": 10 + } + ] + ''' + With forced --format JSON, we can fetch only the result value reliably and compare against the snapshot. + """ + myjson = json.loads(output)[0] + return snapshot.assert_match(myjson.values()) + + @pytest.mark.e2e def test_full_lifecycle_with_codegen( snowcli, test_root_path, project_directory, snapshot @@ -100,9 +129,7 @@ def test_full_lifecycle_with_codegen( text=True, ) - assert result.stderr == "" assert result.returncode == 0 - assert_dir_snapshot(project_dir, snapshot) app_name_and_schema = f"{app_name}.ext_code_schema" @@ -130,14 +157,14 @@ def test_full_lifecycle_with_codegen( config_path=config_path, snowcli=snowcli, ) - snapshot.assert_match(output) + assert_snapshot_match_with_query_result(output, snapshot) output = subprocess_check_output_with( sql_stmt=f"select {app_name_and_schema}.py_echo_fn('test')", config_path=config_path, snowcli=snowcli, ) - snapshot.assert_match(output) + assert_snapshot_match_with_query_result(output, snapshot) # User wrote ext code using codegen feature output = subprocess_check_output_with( @@ -145,28 +172,28 @@ def test_full_lifecycle_with_codegen( config_path=config_path, snowcli=snowcli, ) - snapshot.assert_match(output) + assert_snapshot_match_with_query_result(output, snapshot) output = subprocess_check_output_with( sql_stmt=f"select {app_name_and_schema}.echo_fn_2('test')", config_path=config_path, snowcli=snowcli, ) - snapshot.assert_match(output) + assert_snapshot_match_with_query_result(output, snapshot) output = subprocess_check_output_with( sql_stmt=f"select {app_name_and_schema}.echo_fn_4('test')", config_path=config_path, snowcli=snowcli, ) - snapshot.assert_match(output) + assert_snapshot_match_with_query_result(output, snapshot) output = subprocess_check_output_with( sql_stmt=f"call {app_name_and_schema}.add_sp(1, 2)", config_path=config_path, snowcli=snowcli, ) - snapshot.assert_match(output) + assert_snapshot_match_with_query_result(output, snapshot) # code gen UDAF output = subprocess_check_output_with( @@ -174,7 +201,7 @@ def test_full_lifecycle_with_codegen( config_path=config_path, snowcli=snowcli, ) - snapshot.assert_match(output) + assert_snapshot_match_with_query_result(output, snapshot) # code gen UDTF output = subprocess_check_output_with( @@ -208,7 +235,7 @@ def test_full_lifecycle_with_codegen( config_path=config_path, snowcli=snowcli, ) - snapshot.assert_match(output) + assert_snapshot_match_with_query_result(output, snapshot) # UDTF should exist as only its deploy/root file was de-annotated, but the source should be discovered always output = subprocess_check_output_with( @@ -224,7 +251,7 @@ def test_full_lifecycle_with_codegen( config_path=config_path, snowcli=snowcli, ) - snapshot.assert_match(output) + assert_snapshot_match_with_query_result(output, snapshot) finally: # teardown is idempotent, so we can execute it again with no ill effects