[NADE] Enable Code Generation by default in Snow App Bundle #1167
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sfc-gh-bgoel
force-pushed
the
bgoel-use-codegen-always
branch
4 times, most recently
from
e29182a
to
d3d2184
Compare
sfc-gh-bgoel
changed the title
src/snowflake/cli/plugins/nativeapp/codegen/snowpark/python_processor.py
Outdated
Show resolved
Hide resolved
| @@ -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 error:\n {completed_process.stderr}" | |||
| f"Could not fetch Snowpark objects from {py_file} due to the following Snowpark-internal error:\n {completed_process.stderr}" | |||
There was a problem hiding this comment.
errors here aren't necessarily snowpark
There was a problem hiding this comment.
My understanding is that any sandbox or execution environment based errors have been captured in lines 130 to 139. So if we get to this stage, we must have invoked the jinja file and then received a response accordingly. Is that incorrect?
Either way, I can remove the "Snowpark-internal" phrase.
There was a problem hiding this comment.
Errors that can go to stderr include bugs in our driver and just Python errors in user code. Labelling them all as Snowpark would be misleading.
src/snowflake/cli/plugins/nativeapp/codegen/snowpark/callback_source.py.jinja
Show resolved
Hide resolved
src/snowflake/cli/plugins/nativeapp/codegen/snowpark/callback_source.py.jinja
Outdated
Show resolved
Hide resolved
src/snowflake/cli/plugins/nativeapp/codegen/snowpark/python_processor.py
Show resolved
Hide resolved
sfc-gh-bgoel
commented
Jun 12, 2024
src/snowflake/cli/plugins/nativeapp/codegen/snowpark/callback_source.py.jinja
Show resolved
Hide resolved
src/snowflake/cli/plugins/nativeapp/codegen/snowpark/python_processor.py
Show resolved
Hide resolved
sfc-gh-bgoel
force-pushed
the
bgoel-symlink-to-cp-r
branch
from
20291df
to
87b07a9
Compare
sfc-gh-bgoel
force-pushed
the
bgoel-use-codegen-always
branch
2 times, most recently
from
810a40b
to
a6f2fb0
Compare
sfc-gh-bdufour
previously approved these changes
Jun 13, 2024
commit for Mike's documentation
…import, introduced fn which relies on another in the same file
…sses 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
sfc-gh-bgoel
force-pushed
the
bgoel-use-codegen-always
branch
from
160fa78
to
b429cf6
Compare
sfc-gh-bdufour
approved these changes
Jun 13, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Pre-review checklist
Changes description
Snowpark Annotation Processing in Native Apps through Code Generation of SQL is hidden behind a feature flag up until 2.4.0. But with 2.5.0, we want to make this widely available, without any feature flags. This PR removes the condition, and also adds an E2E test that ensures we get to a working app state.