[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

sfc-gh-lspiegelberg · 2024-11-25T18:03:46Z

tox.ini

@@ -81,6 +81,8 @@ passenv =
    SF_REGRESS_LOGS
    ; Github Actions provided environmental variables


@sfc-gh-azhan this here changed.

duplicate lines in 84 85

To resolve what should I do? I think both are here because of windows/ubuntu differences.

leaving for now as is, we can do follow up PR to resolve/improve.

why? Such a simple change

you just need to move line 84 and 85.

.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

src/snowflake/snowpark/dataframe.py

sfc-gh-azhan · 2024-11-25T23:51:17Z

CHANGELOG.md

@@ -19,6 +19,7 @@
    - `keyType`: keys of the map
    - `valueType`: values of the map
 - Added support for `include_nulls` argument in `DataFrame.unpivot`.
+- Added client-side thin-client mode which can be enabled with `session.ast_enabled=True` (default: `False`).


enable ast does not enable a thin client mode right? We still send sql query to the server.

change to
Added parameter ast_enabled to session for internal usage (default: False)

sfc-gh-azhan · 2024-11-25T23:58:22Z

src/snowflake/snowpark/query_history.py

+    def __init__(
+        self,
+        session: "snowflake.snowpark.session.Session",
+        include_failures: bool = False,


add comment to talk about what include_failures does

@sfc-gh-oplaton can we add a comment here?

src/snowflake/snowpark/modin/plugin/utils/numpy_to_pandas.py

src/snowflake/snowpark/column.py

src/snowflake/snowpark/dataframe.py

sfc-gh-azhan · 2024-11-26T00:10:04Z

src/snowflake/snowpark/dataframe.py

+            self._session._ast_batch.eval(repr)
+
+            # Flush the AST and encode it as part of the query.
+            _, kwargs["_dataframe_ast"] = self._session._ast_batch.flush()


Can we create a constant for "_dataframe_ast"?

add DATAFRAME_AST_PARAMETER="_dataframe_ast"

sfc-gh-azhan · 2024-11-26T00:12:34Z

tests/.gitignore

@@ -1 +1,2 @@
 parameters*.py
+cassettes/


curious what code generate this?

remove this.

sfc-gh-azhan · 2024-11-26T00:16:00Z

tests/integ/conftest.py

+]
+
+
+def _process_response_recording(response):


where do we use _process_response_recording?

remove this. double-check this file.

tests/integ/modin/test_apply_snowpark_python_functions.py

sfc-gh-azhan · 2024-11-26T00:18:43Z

tests/integ/modin/tools/test_to_datetime.py

@@ -597,7 +597,7 @@ def test_to_datetime_pydatetime(self):
    @pytest.mark.parametrize(
        "dt", [np.datetime64("2000-01-01"), np.datetime64("2000-01-02")]
    )
-    @sql_count_checker(query_count=1)
+    @sql_count_checker(query_count=1, join_count=0)


shouldn't change this file

revert this change.

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

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

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-lspiegelberg requested review from sfc-gh-yixie and removed request for sfc-gh-smirzaei, sfc-gh-jdu, sfc-gh-yixie, sfc-gh-rdurrani and sfc-gh-helmeleegy November 25, 2024 22:11

skip io in local testing due to bug

16a21b1

sfc-gh-lspiegelberg requested review from sfc-gh-batur, sfc-gh-evandenberg, sfc-gh-vbudati, sfc-gh-azwiegincew and sfc-gh-oplaton November 25, 2024 22:39

sfc-gh-oplaton reviewed Nov 25, 2024

View reviewed changes

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

sfc-gh-oplaton reviewed Nov 25, 2024

View reviewed changes

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

sfc-gh-azhan reviewed Nov 25, 2024

View reviewed changes

src/snowflake/snowpark/modin/plugin/utils/numpy_to_pandas.py Show resolved Hide resolved

sfc-gh-azhan reviewed Nov 26, 2024

View reviewed changes

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

sfc-gh-azhan reviewed Nov 26, 2024

View reviewed changes

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

sfc-gh-azhan reviewed Nov 26, 2024

View reviewed changes

src/snowflake/snowpark/dataframe.py 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

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 Nov 25, 2024

sfc-gh-azhan Nov 25, 2024

sfc-gh-lspiegelberg Nov 25, 2024

sfc-gh-lspiegelberg Nov 25, 2024

sfc-gh-azhan Nov 25, 2024

sfc-gh-azhan Nov 25, 2024

sfc-gh-azhan Nov 25, 2024

sfc-gh-lspiegelberg Nov 26, 2024

sfc-gh-azhan Nov 25, 2024

sfc-gh-lspiegelberg Nov 26, 2024

sfc-gh-azhan Nov 26, 2024

sfc-gh-lspiegelberg Nov 26, 2024

sfc-gh-azhan Nov 26, 2024

sfc-gh-lspiegelberg Nov 26, 2024

sfc-gh-azhan Nov 26, 2024

sfc-gh-lspiegelberg Nov 26, 2024

sfc-gh-azhan Nov 26, 2024

sfc-gh-lspiegelberg Nov 26, 2024

sfc-gh-lspiegelberg commented Nov 26, 2024

		@@ -81,6 +81,8 @@ passenv =
		SF_REGRESS_LOGS
		; Github Actions provided environmental variables

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

Are you sure you want to change the base?

[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

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

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 26, 2024

sfc-gh-lspiegelberg commented Nov 25, 2024 •

edited

Loading