Skip to content

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

Merged
sfc-gh-yuwang merged 44 commits into
graphite-base/4282from
SNOW-2912540-secret-detector-import
Sep 9, 2026
Merged

SNOW-2912540: add IS_V5_DRIVER constant; use it for SecretDetector/TelemetryData/TelemetryClient/ReauthenticationRequest imports and pandas/pyarrow/numpy resolution#4313
sfc-gh-yuwang merged 44 commits into
graphite-base/4282from
SNOW-2912540-secret-detector-import

Conversation

@sfc-gh-fpawlowski

@sfc-gh-fpawlowski sfc-gh-fpawlowski commented Aug 12, 2026

Copy link
Copy Markdown
Collaborator

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

Stack (via Graphite)

🤖 Generated with Claude Code

@sfc-gh-fpawlowski
sfc-gh-fpawlowski requested a review from a team as a code owner August 12, 2026 16:10
@sfc-gh-fpawlowski
sfc-gh-fpawlowski requested review from sfc-gh-aling, sfc-gh-bkogan and sfc-gh-jzeng and removed request for a team August 12, 2026 16: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.

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

Comment thread src/snowflake/snowpark/_internal/analyzer/analyzer_utils.py Outdated
Comment thread src/snowflake/snowpark/_internal/utils.py Outdated

@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 changed the base branch from SNOW-2912540-is-v5-driver-constant to SNOW-2912540-decouple-oob-telemetry August 17, 2026 18:33
@sfc-gh-fpawlowski
sfc-gh-fpawlowski marked this pull request as ready for review August 17, 2026 18:34
@sfc-gh-fpawlowski sfc-gh-fpawlowski changed the title SNOW-2912540: use IS_V5_DRIVER for SecretDetector import path SNOW-2912540: add IS_V5_DRIVER constant; use it for SecretDetector/TelemetryData imports and pandas/pyarrow/numpy resolution Aug 17, 2026
@codecov-commenter

codecov-commenter commented Aug 17, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 69.04762% with 13 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (graphite-base/4282@7774b01). Learn more about missing BASE report.

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                  @@
##             graphite-base/4282    #4313   +/-   ##
=====================================================
  Coverage                      ?   82.64%           
=====================================================
  Files                         ?      172           
  Lines                         ?    44811           
  Branches                      ?     7692           
=====================================================
  Hits                          ?    37036           
  Misses                        ?     5812           
  Partials                      ?     1963           

☔ 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 force-pushed the SNOW-2912540-secret-detector-import branch from 69382c5 to 07f8f83 Compare August 20, 2026 16:28
@sfc-gh-fpawlowski
sfc-gh-fpawlowski force-pushed the SNOW-2912540-secret-detector-import branch from 07f8f83 to cd9364b Compare August 20, 2026 17:13
@sfc-gh-fpawlowski sfc-gh-fpawlowski changed the title SNOW-2912540: add IS_V5_DRIVER constant; use it for SecretDetector/TelemetryData imports and pandas/pyarrow/numpy resolution SNOW-2912540: add IS_V5_DRIVER constant; use it for SecretDetector/TelemetryData/TelemetryClient/ReauthenticationRequest imports and pandas/pyarrow/numpy resolution Aug 21, 2026
@sfc-gh-fpawlowski
sfc-gh-fpawlowski changed the base branch from SNOW-2912540-decouple-oob-telemetry to graphite-base/4313 August 21, 2026 08:30
sfc-gh-fpawlowski and others added 4 commits September 2, 2026 06:44
…ield imports on IS_V5_DRIVER

network.py doesn't exist in UD at all -- legacy's errors.py/network.py split is
consolidated into errors.py. UD PR #1224 (open, stacked on #1133) adds
ReauthenticationRequest(ProgrammingError) to errors.py and removes network.py
outright, naming Snowpark's import site explicitly as the target. Gate the
import in server_connection.py and its unit test mock, same pattern as every
other IS_V5_DRIVER import in this stack.

TelemetryClient/TelemetryField were imported unconditionally from the
top-level snowflake.connector.telemetry module, which is a stub on v5 (the
real implementation lives in _common.telemetry per UD PR #1106's current
branch). Also fixes this same file's PCTelemetryData import, added in an
earlier commit against _internal.telemetry -- that class moved to _common
too on the same #1106 branch since that commit landed, so it was already
stale for the identical reason.

Adds test coverage for _missing_pandas() (added earlier in this stack to
replace direct MissingPandas() construction), which had zero coverage after
test__pandas_importer() was deleted alongside _pandas_importer() itself.
… import

from snowflake.connector.options import installed_pandas is unconditional in
tests/integ/test_function.py, test_cte.py, scala/test_datatype_suite.py, and
scala/test_update_delete_merge_suite.py -- ModuleNotFoundError once UD deletes
options.py outright (confirmed: f61156c7a, ancestor of SNOW-2912540-extras-to-common's
current tip, already relied on elsewhere in this stack).

Swap to snowflake.snowpark._internal.utils, which already re-exports
installed_pandas correctly gated on IS_V5_DRIVER internally (from this PR's
earlier _pandas_importer()-removal commit) -- no IS_V5_DRIVER awareness needed
in these test files themselves.
…s.py

_internal/utils.py's IS_V5_DRIVER-gated block (MissingOptionalDependency,
ModuleLikeObject, pandas, pyarrow, installed_pandas, installed_pyarrow,
_missing_pandas()) was a self-contained concern mirroring the connector's own
options.py/_common.extras 1:1, buried in an already-large kitchen-sink file.
Moved to a dedicated module, mirroring mock/_options.py's existing role as
the scoped equivalent for the local-testing side.

options.py computes its own IS_V5_DRIVER independently rather than importing
it from utils.py, since utils.py itself needs names back from options.py
(MissingOptionalDependency/ModuleLikeObject/installed_pandas, used by its
modin-optional-dependency code) -- importing in both directions would be
circular. One-line duplication, avoids import-order fragility entirely.

Updated all 17 consumers (found types.py's multi-line import via a
regex-based sweep after a naive single-line grep missed it) to import these
names from _internal.options instead. mock/_options.py and
event_table_telemetry.py keep their other _internal.utils imports
(IS_V5_DRIVER, parse_table_name) unchanged.
…ield

Reuse the canonical IS_V5_DRIVER from _internal.options instead of
redefining it in utils, and gate the leftover Modin connector.telemetry
import on the same flag as Snowpark telemetry.

Co-authored-by: Filip Pawłowski <sfc-gh-fpawlowski@users.noreply.github.com>
@cursor
cursor Bot force-pushed the SNOW-2912540-secret-detector-import branch from 5f3a1eb to 64d0102 Compare September 2, 2026 06:45
@snowflakedb snowflakedb deleted a comment from cursor Bot Sep 2, 2026
cursoragent and others added 16 commits September 2, 2026 17:39
flake8 reports unused IS_V5_DRIVER on the `from` line, so a noqa on the
name itself never applied and Check linting failed from this PR upstack.

Co-authored-by: Filip Pawłowski <sfc-gh-fpawlowski@users.noreply.github.com>
Moving noqa to the from-import line was not enough for Check linting on
this PR. Bind the options module constant to a local name instead of an
unused import.

Co-authored-by: Filip Pawłowski <sfc-gh-fpawlowski@users.noreply.github.com>
pre-commit insert-license failed Check linting from this PR through #4313.

Co-authored-by: Filip Pawłowski <sfc-gh-fpawlowski@users.noreply.github.com>
…secret-detector-import

Co-authored-by: Filip Pawłowski <sfc-gh-fpawlowski@users.noreply.github.com>
…move-connector-shims

Co-authored-by: Filip Pawłowski <sfc-gh-fpawlowski@users.noreply.github.com>
…inline-pandas-staging-helpers

Co-authored-by: Filip Pawłowski <sfc-gh-fpawlowski@users.noreply.github.com>
…912540-decouple-oob-telemetry

Co-authored-by: Filip Pawłowski <sfc-gh-fpawlowski@users.noreply.github.com>
…secret-detector-import

Co-authored-by: Filip Pawłowski <sfc-gh-fpawlowski@users.noreply.github.com>
…move-connector-shims

Co-authored-by: Filip Pawłowski <sfc-gh-fpawlowski@users.noreply.github.com>
…inline-pandas-staging-helpers

Co-authored-by: Filip Pawłowski <sfc-gh-fpawlowski@users.noreply.github.com>
…912540-decouple-oob-telemetry

Co-authored-by: Filip Pawłowski <sfc-gh-fpawlowski@users.noreply.github.com>
…secret-detector-import

Co-authored-by: Filip Pawłowski <sfc-gh-fpawlowski@users.noreply.github.com>
Base automatically changed from SNOW-2912540-decouple-oob-telemetry to graphite-base/4282 September 9, 2026 16:51
@sfc-gh-yuwang
sfc-gh-yuwang added this pull request to stack #4349 September 9, 2026 16:52
Resolve import conflicts by keeping this PR's changes:
- analyzer_utils.py: import pyarrow from snowflake.snowpark._internal.options
- mock/_telemetry.py: drop connector secret_detector/telemetry_oob imports
  (local ._secret_detector.SecretDetector is used; TelemetryService unused)
@sfc-gh-yuwang
sfc-gh-yuwang merged commit 6d49d2c into graphite-base/4282 Sep 9, 2026
6 of 8 checks passed
@sfc-gh-yuwang
sfc-gh-yuwang deleted the SNOW-2912540-secret-detector-import branch September 9, 2026 18:15
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.

4 participants