Skip to content

Conversation

@sfc-gh-rdurrani
Copy link
Contributor

@sfc-gh-rdurrani sfc-gh-rdurrani commented Jan 10, 2025

  1. Which Jira issue is this PR addressing? Make sure that there is an accompanying issue to your PR.

    Fixes SNOW-1871355

  2. 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
  3. Please describe how your code solves the related issue.

Having COMPATIBLE_WITH_MODIN defined in utils.py causes conftest to fail some stored procedure Snowfort tests when the version of snowpark-python imported by the stored procedure is older than the version of the tests being run (the tests files are cloned directly from GitHub and seem to pull the latest version, whereas some stored procedures are run in older python versions and have older versions of snowpark-python). Moving the definition of COMPATIBLE_WITH_MODIN into conftest avoids this issue and maintains backwards compatibility.

The failure is because COMPATIBLE_WITH_MODIN is not defined in snowflake/snowpark/_internal/utils.py for version 1.25.0 which is what the stored proc is running, since it is on python 3.8, and that's the latest version of snowpark-python available for 3.8.

@sfc-gh-rdurrani sfc-gh-rdurrani requested review from a team as code owners January 10, 2025 20:38
@sfc-gh-rdurrani sfc-gh-rdurrani added NO-CHANGELOG-UPDATES This pull request does not need to update CHANGELOG.md NO-PANDAS-CHANGEDOC-UPDATES This PR does not update Snowpark pandas docs labels Jan 10, 2025
Copy link
Contributor

@sfc-gh-jkew sfc-gh-jkew left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change makes sense. Better than the original code anyways because that variable is test only anyways.

Copy link
Contributor

@sfc-gh-jjiao sfc-gh-jjiao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

@sfc-gh-joshi sfc-gh-joshi enabled auto-merge (squash) January 15, 2025 22:23
@sfc-gh-joshi sfc-gh-joshi merged commit 4fb3b33 into main Jan 15, 2025
48 of 51 checks passed
@sfc-gh-joshi sfc-gh-joshi deleted the rdurrani-SNOW-1871355 branch January 15, 2025 23:10
@github-actions github-actions bot locked and limited conversation to collaborators Jan 15, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

NO-CHANGELOG-UPDATES This pull request does not need to update CHANGELOG.md NO-PANDAS-CHANGEDOC-UPDATES This PR does not update Snowpark pandas docs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants