-
Notifications
You must be signed in to change notification settings - Fork 156
SNOW-2912540: define ASYNC_RETRY_PATTERN and compat.OK locally - #4282
SNOW-2912540: define ASYNC_RETRY_PATTERN and compat.OK locally #4282sfc-gh-fpawlowski wants to merge 9 commits into
Conversation
Codecov Report
✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.24%. Comparing base (912c2ee) to head (ef63724).
Additional details and impacted files
@@ Coverage Diff @@ ## graphite-base/4282 #4282 +/- ## =================================================== Coverage 95.24% 95.24% =================================================== Files 171 171 Lines 44754 44755 +1 Branches 7685 7685 =================================================== + Hits 42628 42629 +1 Misses 1339 1339 Partials 787 787
☔ 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.
42128db to
a87e53a
Compare
There was a problem hiding this comment.
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.
Warning
This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more
- SNOW-2912540: decouple LocalTestOOBTelemetryService from connector.telemetry_oob #4309 Graphite: 2 dependent PRs (#4310 Graphite, #4313 Graphite)
- SNOW-2912540: inline _create_temp_stage and _create_temp_file_format into analyzer_utils #4308 Graphite
- SNOW-2912540: define ASYNC_RETRY_PATTERN and compat.OK locally #4282 Graphite 👈 (View in Graphite)
- SNOW-2912540: translate create_temp_table=True to table_type in AST encoding (rebased onto main) #4307 Graphite
main
This stack of pull requests is managed by Graphite. Learn more about stacking.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
b14c893 to
74db794
Compare
ff905f3 to
0ce0042
Compare
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
d9de3cb to
4d80196
Compare
a592c7d to
b245fcd
Compare
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
...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>
4d80196 to
9d96c9c
Compare
b245fcd to
9ab3e6a
Compare
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
...move-connector-shims Co-authored-by: Filip Pawłowski <sfc-gh-fpawlowski@users.noreply.github.com>
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
...move-connector-shims Co-authored-by: Filip Pawłowski <sfc-gh-fpawlowski@users.noreply.github.com>
There was a problem hiding this comment.
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.
...move-connector-shims
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
Merge activity
- Sep 3, 8:07 AM UTC: A user started a stack merge that includes this pull request via Graphite.
- Sep 3, 8:08 AM UTC: Graphite couldn't merge this pull request because a downstack PR SNOW-2912540: translate create_temp_table=True to table_type in AST encoding (rebased onto main) #4307 failed to merge.
- Sep 3, 8:09 AM UTC: A user started a stack merge that includes this pull request via Graphite.
- Sep 3, 8:10 AM UTC: Graphite couldn't merge this pull request because a downstack PR SNOW-2912540: translate create_temp_table=True to table_type in AST encoding (rebased onto main) #4307 failed to merge.
- Sep 4, 5:41 AM UTC: A user started a stack merge that includes this pull request via Graphite.
- Sep 4, 5:41 AM UTC: Graphite couldn't merge this pull request because a downstack PR SNOW-2912540: translate create_temp_table=True to table_type in AST encoding (rebased onto main) #4307 failed to merge.
Uh oh!
There was an error while loading. Please reload this page.
Removes two imports from snowflake-connector-python that the Universal Driver connector no longer provides as shims.
async_job.py:ASYNC_RETRY_PATTERNdefined locally (was imported fromconnector.cursor)mock/_telemetry.py:from http.client import OK(was imported fromconnector.compat)Companion changes in snowflake-eng/universal-driver PR #512 and #513.
Checklist
Stack (via Graphite)
🤖 Generated with Claude Code