-
Notifications
You must be signed in to change notification settings - Fork 156
SNOW-2912540: mock _connect on v5 so local testing skips UD connection_init - #4326
SNOW-2912540: mock _connect on v5 so local testing skips UD connection_init #4326sfc-gh-fpawlowski wants to merge 10 commits into
Conversation
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: remap write_pandas missing-table errors via ProgrammingError.raw_msg #4328 Graphite
- SNOW-2912540: mock _connect on v5 so local testing skips UD connection_init #4326 Graphite 👈 (View in Graphite)
- SNOW-2912540: mock-patch try_add_log_to_batch instead of reading _log_batch #4321 Graphite: 1 other dependent PR (#4322 Graphite)
- SNOW-2912540: use IS_V5_DRIVER for cursor request_id access #4314 Graphite: 1 other dependent PR (#4317 Graphite)
- SNOW-2912540: add IS_V5_DRIVER constant; use it for SecretDetector/TelemetryData/TelemetryClient/ReauthenticationRequest imports and pandas/pyarrow/numpy resolution #4313 Graphite
- SNOW-2912540: decouple LocalTestOOBTelemetryService from connector.telemetry_oob #4309 Graphite: 1 other dependent PR (#4310 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
- 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.
...n_init Co-authored-by: Cursor <cursoragent@cursor.com>
2257b69 to
f1ab28e
Compare
e167ad3 to
f0eb42e
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.
...ck-ud-connect-hook Co-authored-by: Filip Pawłowski <sfc-gh-fpawlowski@users.noreply.github.com>
Class-body if/else method defs were rewritten by black in CI. Define connect() once and alias _connect on IS_V5_DRIVER so UD __init__ hits the same no-op as the legacy connect() override. 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.
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.
...ck-ud-connect-hook 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.
...ck-ud-connect-hook 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.
...ck-ud-connect-hook 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.
...ck-ud-connect-hook 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.
Codecov Report
✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.21%. Comparing base (cc430d8) to head (17b2848).
Additional details and impacted files
@@ Coverage Diff @@ ## SNOW-2912540-log-batch-mock-patch #4326 +/- ## ================================================================== Coverage 95.21% 95.21% ================================================================== Files 172 172 Lines 44811 44811 Branches 7692 7692 ================================================================== Hits 42668 42668 Misses 1351 1351 Partials 792 792
☔ 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.
...ck-ud-connect-hook
Uh oh!
There was an error while loading. Please reload this page.
Summary
MockedSnowflakeConnection(Local Testing's mock connection class) subclasses the realSnowflakeConnectionand overridesconnect()to a no-op, avoiding a real network connection attempt during local testing sessions.On v5 (UD),
Connection.__init__calls the private_connect()internally instead of the publicconnect()the legacy v4 connector called. Since the mock override was only namedconnect, UD's real_connect()ran unmocked onIS_V5_DRIVER=True, attempting a genuine connection attempt requiring real account credentials — surfacing as an account/credentials error in local-testing sessions that should never touch the network at all.Fix: on v5, alias
_connect = connectso the existing mock override is actually invoked by UD's__init__path.Known gap: no dedicated test exercises this specific
IS_V5_DRIVER=Truecode path (unverifiable without a UD wheel installed, same caveat as the rest of this stack). Flagging for reviewer awareness rather than holding up this draft.Stacked on #4321.
Test plan
flake8/blackcleantests/unit/+tests/mock/suite passes for v4 (2916 passed) — this branch doesn't change v4 behavior at allIS_V5_DRIVER=Truepath itself🤖 Generated with Claude Code