Skip to content

SNOW-1882338 Fix AST to_snowpark_pandas test in nop testing mode#2863

Merged
sfc-gh-vbudati merged 1 commit intomainfrom
vbudati/SNOW-1882338-support-tmp-read-only-table
Jan 15, 2025
Merged

SNOW-1882338 Fix AST to_snowpark_pandas test in nop testing mode#2863
sfc-gh-vbudati merged 1 commit intomainfrom
vbudati/SNOW-1882338-support-tmp-read-only-table

Conversation

@sfc-gh-vbudati
Copy link
Contributor

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

Fixes SNOW-1882338

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

Add logic to the no-op AST testing code to create a temp read-only table for to_snowpark_pandas. This way to_snowpark_pandas works with both nop mode and local_testing_mode.

@sfc-gh-vbudati sfc-gh-vbudati added NO-CHANGELOG-UPDATES This pull request does not need to update CHANGELOG.md snowpark-server Server-side snowpark work labels Jan 14, 2025
@sfc-gh-vbudati sfc-gh-vbudati requested review from a team as code owners January 14, 2025 21:35
@github-actions github-actions bot added the local testing Local Testing issues/PRs label Jan 14, 2025
source_plan = plan.source_plan

if hasattr(source_plan, "execution_queries"):
# If temp read-only table, explicitly create it.
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: maybe add a simple unit/integ test for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think that's necessary/it would be redundant since the to_snowpark_pandas.test test will fail if there's something wrong with this generation

Copy link
Contributor

@sfc-gh-lspiegelberg sfc-gh-lspiegelberg left a comment

Choose a reason for hiding this comment

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

lgtm

@sfc-gh-vbudati sfc-gh-vbudati merged commit 4d29402 into main Jan 15, 2025
49 checks passed
@sfc-gh-vbudati sfc-gh-vbudati deleted the vbudati/SNOW-1882338-support-tmp-read-only-table branch January 15, 2025 00:12
@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

local testing Local Testing issues/PRs NO-CHANGELOG-UPDATES This pull request does not need to update CHANGELOG.md snowpark-server Server-side snowpark work

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants