Skip to content

SNOW-2912540: define ASYNC_RETRY_PATTERN and compat.OK locally - #4282

Open
sfc-gh-fpawlowski wants to merge 6 commits into
create-temp-table-ast-fix-on-mainfrom
SNOW-2912540-remove-connector-shims
Open

SNOW-2912540: define ASYNC_RETRY_PATTERN and compat.OK locally#4282
sfc-gh-fpawlowski wants to merge 6 commits into
create-temp-table-ast-fix-on-mainfrom
SNOW-2912540-remove-connector-shims

Conversation

@sfc-gh-fpawlowski

@sfc-gh-fpawlowski sfc-gh-fpawlowski commented Jul 9, 2026

Copy link
Copy Markdown

Removes two imports from snowflake-connector-python that the Universal Driver connector no longer provides as shims.

  • async_job.py: ASYNC_RETRY_PATTERN defined locally (was imported from connector.cursor)
  • mock/_telemetry.py: from http.client import OK (was imported from connector.compat)

Companion changes in snowflake-eng/universal-driver PR #512 and #513.

Checklist

  • If adding any arguments to public Snowpark APIs or creating new public Snowpark APIs, I acknowledge that I have ensured my changes include AST support.
  • I acknowledge that I have ensured my changes to be thread-safe

Stack (via Graphite)

🤖 Generated with Claude Code

@github-actions github-actions Bot added the local testing Local Testing issues/PRs label Jul 9, 2026
@codecov-commenter

codecov-commenter commented Jul 9, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (create-temp-table-ast-fix-on-main@4d80196). Learn more about missing BASE report.

Additional details and impacted files
@@                         Coverage Diff                          @@
##             create-temp-table-ast-fix-on-main    #4282   +/-   ##
====================================================================
  Coverage                                     ?   95.46%           
====================================================================
  Files                                        ?      171           
  Lines                                        ?    44719           
  Branches                                     ?     7676           
====================================================================
  Hits                                         ?    42693           
  Misses                                       ?     1253           
  Partials                                     ?      773           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@sfc-gh-fpawlowski
sfc-gh-fpawlowski marked this pull request as ready for review July 22, 2026 16:45
@sfc-gh-fpawlowski
sfc-gh-fpawlowski requested review from a team as code owners July 22, 2026 16:45
@sfc-gh-fpawlowski
sfc-gh-fpawlowski force-pushed the SNOW-2912540-remove-connector-shims branch from 42128db to a87e53a Compare August 6, 2026 09:09
@sfc-gh-fpawlowski
sfc-gh-fpawlowski changed the base branch from main to create-temp-table-ast-fix-on-main August 6, 2026 09:10

@snowflake-security-bot snowflake-security-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Snowflake Security Review

Security grade: A — Passed

This PR was classified as LOW risk by the automated pre-screen.

sfc-gh-fpawlowski commented Aug 6, 2026

Copy link
Copy Markdown
Author

@snowflake-security-bot snowflake-security-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Snowflake Security Review

Security grade: A — Passed

This PR was classified as LOW risk by the automated pre-screen.

@snowflake-security-bot snowflake-security-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Snowflake Security Review

Security grade: A — Passed

This PR was classified as LOW risk by the automated pre-screen.

@sfc-gh-fpawlowski
sfc-gh-fpawlowski force-pushed the SNOW-2912540-remove-connector-shims branch from b14c893 to 74db794 Compare August 20, 2026 16:27
@sfc-gh-fpawlowski
sfc-gh-fpawlowski force-pushed the create-temp-table-ast-fix-on-main branch from ff905f3 to 0ce0042 Compare August 20, 2026 16:27

@snowflake-security-bot snowflake-security-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Snowflake Security Review

Security grade: A — Passed

This PR was classified as LOW risk by the automated pre-screen.

@snowflake-security-bot snowflake-security-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Snowflake Security Review

Security grade: A — Passed

This PR was classified as LOW risk by the automated pre-screen.

@snowflake-security-bot snowflake-security-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Snowflake Security Review

Security grade: A — Passed

This PR was classified as LOW risk by the automated pre-screen.

@snowflake-security-bot snowflake-security-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Snowflake Security Review

Security grade: A — Passed

This PR was classified as LOW risk by the automated pre-screen.

@snowflake-security-bot snowflake-security-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Snowflake Security Review

Security grade: A — Passed

This PR was classified as LOW risk by the automated pre-screen.

sfc-gh-fpawlowski and others added 6 commits August 21, 2026 08:22
…ncoding

The deprecated `create_temp_table` parameter was being emitted to the
proto AST as a separate boolean field even though the runtime already
translates it to `table_type="temporary"`. This meant the AST decoder
had to handle two representations for the same thing.

Fix: move the deprecation coercion before the AST block in save_as_table
so `table_type` is already resolved when emitted; remove the deprecated
field from both AST emission sites (WriteTable and WritePandas); update
the internal cache_result mock path to pass table_type="temp" directly;
mark the proto fields as deprecated.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
These constants were previously imported from snowflake-connector-python.
The Universal Driver connector no longer owns them (it has no internal use
for either symbol), so define them locally to remove the coupling.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The Universal Driver connector's constants module won't provide
ENV_VAR_PARTNER either (per review discussion on
snowflake-eng/universal-driver#512), so define it locally in
server_connection.py alongside the other now-local backward
compatibility constants (ASYNC_RETRY_PATTERN, compat.OK).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@sfc-gh-fpawlowski
sfc-gh-fpawlowski force-pushed the create-temp-table-ast-fix-on-main branch from d9de3cb to 4d80196 Compare August 21, 2026 08:24
@sfc-gh-fpawlowski
sfc-gh-fpawlowski force-pushed the SNOW-2912540-remove-connector-shims branch from a592c7d to b245fcd Compare August 21, 2026 08:24

@snowflake-security-bot snowflake-security-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Snowflake Security Review

Security grade: A — Passed

This PR was classified as LOW risk by the automated pre-screen.

@snowflake-security-bot snowflake-security-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Snowflake Security Review

Security grade: A — Passed

This PR was classified as LOW risk by the automated pre-screen.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

local testing Local Testing issues/PRs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants