[SNOW-1489960, SNOW-1491199] Snowpark python API client-side changes for IR encoding to protobuf #2674

sfc-gh-lspiegelberg · 2024-11-25T17:43:50Z

Main commit for SNOW-1489960 bringing in changes to encode Snowpark python API to protobuf. A detailed commit history can be found in https://github.com/snowflakedb/snowpark-python/tree/ls-SNOW-1491199-merge-phase0-server-side.

Fill out the following pre-review checklist:
- I am adding a new automated test(s) to verify correctness of my new code
  - If this test skips Local Testing mode, I'm requesting review from @snowflakedb/local-testing
- I am adding new logging messages
- I am adding a new telemetry message
- I am adding new credentials
- I am adding a new dependency
- If this is a new feature/behavior, I'm adding the Local Testing parity changes.
- I acknowledge that I have ensured my changes to be thread-safe. Follow the link for more information: Thread-safe Developer Guidelines
Please describe how your code solves the related issue.

Please write a short description of how your code change solves the related issue.

Co-authored-by: Arthur Zwiegincew [email protected]
Co-authored-by: Ovidiu Platon [email protected]
Co-authored-by: Hemit Shah [email protected]
Co-authored-by: Varnika Budati [email protected]
Co-authored-by: Eric Vandenberg [email protected]
Co-authored-by: Andong Zhan [email protected]

setup.py

tox.ini

.github/workflows/precommit.yml

scripts/tox_install_cmd.sh

setup.py

…me (#2678) SNOW-1787415 Fix udf, udaf, sproc functions with registered object name

src/conftest.py

tests/integ/conftest.py

tests/integ/modin/test_apply_snowpark_python_functions.py

tests/integ/modin/tools/test_to_datetime.py

sfc-gh-lspiegelberg · 2024-11-26T01:15:47Z

Add coverage to AST gates

      - name: Combine coverages
        run: python -m tox -e coverage --skip-missing-interpreters false
        shell: bash
        env:
          SNOWFLAKE_IS_PYTHON_RUNTIME_TEST: 1
      - uses: actions/upload-artifact@v4
        with:
          include-hidden-files: true
          name: coverage_${{ matrix.os }}-${{ matrix.python-version }}-${{ matrix.cloud-provider }}
          path: |
            .tox/.coverage
            .tox/coverage.xml

src/snowflake/snowpark/_internal/analyzer/table_function.py

src/snowflake/snowpark/_internal/compiler/repeated_subquery_elimination.py

sfc-gh-jrose

Local testing section is looking mostly good to me.

src/snowflake/snowpark/mock/_plan.py

src/snowflake/snowpark/mock/_snowflake_data_type.py

sfc-gh-jkew

I don't think this needs an intensive review - most of this will work itself out over the next day anyways.

src/snowflake/snowpark/_internal/analyzer/table_merge_expression.py

sfc-gh-aalam · 2024-11-26T20:50:36Z

src/snowflake/snowpark/_internal/proto/update-from-devvm.sh

+# Cloud workspace ssh is set up via ~/.local/share/sfcli/ssh_config (or simply provide your workspace ID)
+# For this script to succeed with the DevVM, you need to configure the SSH devvm host according to
+# https://snowflakecomputing.atlassian.net/wiki/spaces/EN/pages/3019015180/DevVM+Troubleshooting#Symptom:-Need-to-ssh-into-the-certified-DevVM
+# I.e., add the following lines to ~/.ssh/config


can we move this info from here to a wiki directly? seems sensitive to me.

sure, which information should be redacted in your opinion?

ssh and sfcli related information

https://snowflakecomputing.atlassian.net/browse/SNOW-1842532

src/snowflake/snowpark/_internal/utils.py

src/snowflake/snowpark/column.py

sfc-gh-aalam · 2024-11-26T22:33:11Z

src/snowflake/snowpark/session.py

+
+            >>> import gc
+            >>>
+            >>> def f(session: Session) -> str:


this is also unrealted to your change it seems

https://snowflakecomputing.atlassian.net/browse/SNOW-1842532

sfc-gh-aalam · 2024-11-26T22:35:11Z

src/snowflake/snowpark/session.py

+                if isinstance(self._conn, MockServerConnection):
+                    # TODO: Implement here write_pandas correctly.
+                    success, ci_output = True, []


we already raise a not implemented error earlier. Does adding an assert here make more sense?

https://snowflakecomputing.atlassian.net/browse/SNOW-1842541

src/snowflake/snowpark/table.py

src/snowflake/snowpark/udf.py

squash merge staging branch changes

fa6bf42

github-actions bot added local testing Local Testing issues/PRs snowpark-pandas labels Nov 25, 2024

sfc-gh-lspiegelberg added the NO-PANDAS-CHANGEDOC-UPDATES This PR does not update Snowpark pandas docs label Nov 25, 2024

cleanup, track delta changes in SNOW-1826001

b7d650f

sfc-gh-lspiegelberg commented Nov 25, 2024

View reviewed changes

setup.py Show resolved Hide resolved

sfc-gh-lspiegelberg commented Nov 25, 2024

View reviewed changes

tox.ini Outdated Show resolved Hide resolved

sfc-gh-azhan reviewed Nov 25, 2024

View reviewed changes

.github/workflows/precommit.yml Show resolved Hide resolved

sfc-gh-azhan reviewed Nov 25, 2024

View reviewed changes

.github/workflows/precommit.yml Show resolved Hide resolved

sfc-gh-azhan reviewed Nov 25, 2024

View reviewed changes

scripts/tox_install_cmd.sh Outdated Show resolved Hide resolved

sfc-gh-azhan reviewed Nov 25, 2024

View reviewed changes

setup.py Outdated Show resolved Hide resolved

sfc-gh-azhan reviewed Nov 25, 2024

View reviewed changes

setup.py Outdated Show resolved Hide resolved

sfc-gh-oplaton and others added 4 commits November 25, 2024 11:35

Restore Dataframe.unpivot parameter lost during 3-way merge

76ed3c3

SNOW-1787415 Fix udf, udaf, sproc functions with registered object na…

088de84

…me (#2678) SNOW-1787415 Fix udf, udaf, sproc functions with registered object name

fix test

ca18771

update changelog

1363f99

sfc-gh-azhan reviewed Nov 25, 2024

View reviewed changes

src/conftest.py Outdated Show resolved Hide resolved

sfc-gh-lspiegelberg changed the title ~~[DRAFT] squash merge staging branch changes~~ SNOW-1489960 Snowpark python API client-side changes for IR encoding to protobuf Nov 25, 2024

sfc-gh-lspiegelberg changed the title ~~SNOW-1489960 Snowpark python API client-side changes for IR encoding to protobuf~~ [SNOW-1489960, SNOW-1491199] Snowpark python API client-side changes for IR encoding to protobuf Nov 25, 2024

sfc-gh-lspiegelberg added 3 commits November 25, 2024 14:03

restore modin main changes

d5e4c06

Merge branch 'main' into ls-SNOW-1491199-merge-phase0

0c077be

address Andong's feedback

7ed7cbf

sfc-gh-lspiegelberg marked this pull request as ready for review November 25, 2024 22:10

sfc-gh-lspiegelberg requested review from a team as code owners November 25, 2024 22:10

sfc-gh-lspiegelberg requested review from sfc-gh-rdurrani, sfc-gh-helmeleegy, sfc-gh-smirzaei and sfc-gh-jdu November 25, 2024 22:10

sfc-gh-azhan reviewed Nov 26, 2024

View reviewed changes

tests/integ/conftest.py Outdated Show resolved Hide resolved

sfc-gh-azhan reviewed Nov 26, 2024

View reviewed changes

tests/integ/modin/test_apply_snowpark_python_functions.py Show resolved Hide resolved

sfc-gh-azhan reviewed Nov 26, 2024

View reviewed changes

tests/integ/modin/tools/test_to_datetime.py Outdated Show resolved Hide resolved

Add a docstring for the AstListener initializer

2becb78

sfc-gh-jkew reviewed Nov 26, 2024

View reviewed changes

src/snowflake/snowpark/_internal/analyzer/table_function.py Show resolved Hide resolved

src/snowflake/snowpark/_internal/compiler/repeated_subquery_elimination.py Outdated Show resolved Hide resolved

sfc-gh-lspiegelberg added 9 commits November 26, 2024 09:38

add ast_enabled line to changelog

46f2f42

replace string field usage for JSON request with parameter

c1ab07d

revert gitignore change

3efbcb3

remove query_count=0

7fe673f

add coverage to precommit

31cc2f9

restore subquery elim file

f2f347d

remove deadcode in integ/conftest

6aa64b3

skip another unstable multithreading test

a8f645d

Merge branch 'main' into ls-SNOW-1491199-merge-phase0

44a44a9

sfc-gh-jrose reviewed Nov 26, 2024

View reviewed changes

remove mock folder from coverage

57862c4

sfc-gh-jkew approved these changes Nov 26, 2024

View reviewed changes

local testing comments

3d8efd3

sfc-gh-azhan approved these changes Nov 26, 2024

View reviewed changes

sfc-gh-jrose approved these changes Nov 26, 2024

View reviewed changes

sfc-gh-lspiegelberg enabled auto-merge (squash) November 26, 2024 21:30

sfc-gh-lspiegelberg merged commit 9f9398b into main Nov 26, 2024
39 of 40 checks passed

sfc-gh-lspiegelberg deleted the ls-SNOW-1491199-merge-phase0 branch November 26, 2024 21:52

github-actions bot locked and limited conversation to collaborators Nov 26, 2024

sfc-gh-aalam requested changes Nov 26, 2024

View reviewed changes

sfc-gh-aalam reviewed Nov 26, 2024

View reviewed changes

src/snowflake/snowpark/table.py Show resolved Hide resolved

sfc-gh-aalam reviewed Nov 26, 2024

View reviewed changes

src/snowflake/snowpark/udf.py Show resolved Hide resolved

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SNOW-1489960, SNOW-1491199] Snowpark python API client-side changes for IR encoding to protobuf #2674

[SNOW-1489960, SNOW-1491199] Snowpark python API client-side changes for IR encoding to protobuf #2674

sfc-gh-lspiegelberg commented Nov 25, 2024 •

edited

Loading

sfc-gh-lspiegelberg commented Nov 26, 2024

sfc-gh-jrose left a comment

sfc-gh-jkew left a comment

sfc-gh-aalam Nov 26, 2024

sfc-gh-lspiegelberg Nov 26, 2024

sfc-gh-aalam Nov 26, 2024

sfc-gh-aalam Dec 2, 2024

sfc-gh-aalam Nov 26, 2024

sfc-gh-aalam Dec 2, 2024

sfc-gh-aalam Nov 26, 2024

sfc-gh-aalam Dec 2, 2024

[SNOW-1489960, SNOW-1491199] Snowpark python API client-side changes for IR encoding to protobuf #2674

[SNOW-1489960, SNOW-1491199] Snowpark python API client-side changes for IR encoding to protobuf #2674

Conversation

sfc-gh-lspiegelberg commented Nov 25, 2024 • edited Loading

sfc-gh-lspiegelberg commented Nov 26, 2024

sfc-gh-jrose left a comment

Choose a reason for hiding this comment

sfc-gh-jkew left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sfc-gh-lspiegelberg commented Nov 25, 2024 •

edited

Loading