Skip to content

SNOW-2912540: add IS_V5_DRIVER constant; use it for SecretDetector/TelemetryData/TelemetryClient/ReauthenticationRequest imports and pandas/pyarrow/numpy resolution - universal core transition - #4358

Merged
sfc-gh-fpawlowski merged 6 commits into
mainfrom
SNOW-2912540-secret-detector-import
Sep 11, 2026
Merged

Conversation

@sfc-gh-fpawlowski

Copy link
Copy Markdown
Collaborator

Re-submitted onto main. The original #4313 was squash-merged into the Graphite stack's synthetic base branch (graphite-base/4282) rather than main, so its changes never reached main. This PR restores the same change, cherry-picked onto current main and verified patch-identical to the original merge commit 6d49d2c38.

Summary

  • Adds a module-level IS_V5_DRIVER: bool = connector_version[0] >= 5 to _internal/utils.py — a single canonical place for code that needs to branch on connector generation (legacy v3/v4 vs Universal Driver v5+). Originally opened as its own PR (SNOW-2912540: add IS_V5_DRIVER constant for version-conditioned connector imports #4310); folded in here since every other change in this PR is gated on it.
  • snowflake.connector.secret_detector does not exist in v5 (UD) — SecretDetector lives at snowflake.connector._common.secret_detector. Fixed with an IS_V5_DRIVER-gated conditional import in mock/_telemetry.py.
  • snowflake.connector.options already provides MissingOptionalDependency, ModuleLikeObject, and pyarrow on both driver generations — no gap to fill. Re-import these from the connector, gated on IS_V5_DRIVER, instead of redefining them locally in _internal/utils.py. installed_pyarrow is now imported directly on v5 (UD PR [Local Testing] SNOW-904981 Support Column bitwise operations and unary minus expression #1151 fixed it to check pyarrow independently instead of mirroring installed_pandas) but still derived locally via isinstance on v4 (not exported there at all). MissingPandas is imported only on v4 (UD PR [Local Testing] SNOW-904981 Support Column bitwise operations and unary minus expression #1151 deleted it outright on v5 — no replacement, per BehaviorDifferences.yaml Fix drop columns of dataframe join #66); a _missing_pandas() helper builds the sentinel with whichever construction style each driver generation supports, so callers don't need their own IS_V5_DRIVER branch.
  • Removes _internal/utils.py's pre-existing _pandas_importer() (a second, independent pandas-resolution attempt that predates this whole effort) in favor of sourcing pandas/installed_pandas from the same IS_V5_DRIVER-gated import as the rest of these names. Dedups mock/_options.py's numpy handling against connector._common.extras.numpy on v5 (confirmed pure duplicate; v4 keeps its own MissingNumpy since v4's options.py has no numpy handling to delegate to). Originally opened as a separate PR (SNOW-2912540: remove _pandas_importer(), dedup mock/_options.py numpy handling #4317); folded in here since the diff was small.
  • _internal/telemetry.py's TelemetryClient/TelemetryField import was completely unconditional, pointing at the top-level snowflake.connector.telemetry module — a real implementation on v4, but a """BACKWARD COMPATIBILITY MODULE ONLY""" stub on v5. Gated on IS_V5_DRIVER: v5 now imports from _common.telemetry, matching UD PR SNOW-946900: Add internal parameter in stored proc registration to allow forcing inline code #1106's current branch (verified directly — _internal/telemetry.py no longer exists there at all, fully moved). This same commit also corrects this file's TelemetryData import, which an earlier commit here pointed at _internal.telemetry — that class moved to _common too on the same SNOW-946900: Add internal parameter in stored proc registration to allow forcing inline code #1106 branch since that earlier fix landed, so it was already stale for the identical reason.
  • server_connection.py imported ReauthenticationRequest unconditionally from snowflake.connector.network — that module doesn't exist in UD at all (legacy's errors.py/network.py split is consolidated into errors.py). UD PR SNOW-1023214: Support date_part argument in last_day #1224 (open, stacked on SNOW-964034 Enable skipped multistmt tests for stored proc #1133) adds ReauthenticationRequest(ProgrammingError) to errors.py and removes network.py outright, naming Snowpark's import site explicitly as the target. Gated the import (and its unit test mock in test_server_connection.py, which had the same unconditional-import bug) on IS_V5_DRIVER.
  • Added test coverage for _missing_pandas() (had zero coverage after test__pandas_importer() was deleted alongside _pandas_importer() itself).
  • Fixed 4 integration test files (tests/integ/test_function.py, tests/integ/test_cte.py, tests/integ/scala/test_datatype_suite.py, tests/integ/scala/test_update_delete_merge_suite.py) that unconditionally imported installed_pandas from snowflake.connector.options — that module is deleted outright in UD PR [Local Testing] SNOW-904981 Support Column bitwise operations and unary minus expression #1151 (confirmed: f61156c7a is an ancestor of SNOW-2912540-extras-to-common's current tip).
  • Extracted the whole IS_V5_DRIVER-gated options/pandas/pyarrow block out of _internal/utils.py into a new, dedicated _internal/options.py — mirrors the connector's own options.py/_common.extras 1:1, and mirrors mock/_options.py's existing role as the scoped equivalent for local testing. Updated all 17 consumers (including types.py, whose multi-line import a naive single-line grep initially missed — caught by a regex-based sweep afterward). options.py computes its own IS_V5_DRIVER rather than importing it from utils.py, since utils.py itself needs names back from options.py (its modin-optional-dependency code) — importing in both directions would be circular.
  • Part of the SNOW-2912540 Graphite stack decoupling Snowpark from the connector compat shim.

Still open: UD PRs #1151, #1152, #1106, #1133, and #1224 are unmerged. This PR's IS_V5_DRIVER=True paths are written against their current source but unverifiable end-to-end until they merge.

Test plan

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

@sfc-gh-fpawlowski
sfc-gh-fpawlowski requested a review from a team as a code owner September 9, 2026 20:14
@sfc-gh-fpawlowski
sfc-gh-fpawlowski requested review from sfc-gh-bkogan, sfc-gh-jzeng and sfc-gh-yuwang and removed request for a team September 9, 2026 20:14
sfc-gh-fpawlowski and others added 3 commits September 11, 2026 12:02
The 'Update branch' merge pulled in main's still-unrefactored
snowflake.connector.secret_detector/telemetry_oob imports alongside this
branch's already-local ones, causing flake8 F401 (unused TelemetryService)
and F811 (duplicate SecretDetector).
…lemetryData/TelemetryClient/ReauthenticationRequest imports and pandas/pyarrow/numpy resolution - universal core transition

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Filip Pawłowski <sfc-gh-fpawlowski@users.noreply.github.com>
Co-authored-by: Yuyang Wang <yuyang.wang@snowflake.com>
@sfc-gh-fpawlowski
sfc-gh-fpawlowski force-pushed the SNOW-2912540-secret-detector-import branch from 0ad62d4 to dd2cba2 Compare September 11, 2026 12:44
Base automatically changed from SNOW-2912540-decouple-oob-telemetry to main September 11, 2026 13:39
@sfc-gh-fpawlowski
sfc-gh-fpawlowski enabled auto-merge (squash) September 11, 2026 13:39
@sfc-gh-fpawlowski
sfc-gh-fpawlowski merged commit 630e6c9 into main Sep 11, 2026
26 of 29 checks passed
@sfc-gh-fpawlowski
sfc-gh-fpawlowski deleted the SNOW-2912540-secret-detector-import branch September 11, 2026 14:25
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 69.04762% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 95.44%. Comparing base (79809ac) to head (dd2cba2).
⚠️ Report is 6 commits behind head on main.

Files with missing lines Patch % Lines
src/snowflake/snowpark/_internal/options.py 56.25% 5 Missing and 2 partials ⚠️
.../snowflake/snowpark/_internal/server_connection.py 50.00% 1 Missing and 1 partial ⚠️
src/snowflake/snowpark/_internal/telemetry.py 33.33% 1 Missing and 1 partial ⚠️
...flake/snowpark/modin/plugin/_internal/telemetry.py 50.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4358      +/-   ##
==========================================
- Coverage   95.48%   95.44%   -0.04%     
==========================================
  Files         172      173       +1     
  Lines       44824    44880      +56     
  Branches     7694     7701       +7     
==========================================
+ Hits        42799    42837      +38     
- Misses       1252     1265      +13     
- Partials      773      778       +5     

☔ 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.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants