SNOW-2912540: add IS_V5_DRIVER constant; use it for SecretDetector/TelemetryData/TelemetryClient/ReauthenticationRequest imports and pandas/pyarrow/numpy resolution - #4313
Open
sfc-gh-fpawlowski wants to merge 13 commits into
Conversation
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[bot]](https://avatars.githubusercontent.com/u/140554735?s=60&v=4){kind=small}
| from snowflake.connector import ProgrammingError | ||
| from snowflake.connector.cursor import SnowflakeCursor | ||
| from snowflake.connector.options import pyarrow | ||
| from snowflake.snowpark._internal.utils import pyarrow |
Author
There was a problem hiding this comment.
should be _internal.options
| from snowflake.snowpark._internal.error_message import SnowparkClientExceptionMessages | ||
|
|
||
|
|
||
| class MissingOptionalDependency: |
Author
There was a problem hiding this comment.
This should be in driver - from there imported here
Author
There was a problem hiding this comment.
Or shouldnt it be from class MissingOptionalDependency in _internal.extras?
This was referenced Aug 13, 2026
Author
|
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.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
sfc-gh-fpawlowski
marked this pull request as draft
August 14, 2026 17:29
3 tasks
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
marked this pull request as ready for review
August 17, 2026 18:34
sfc-gh-fpawlowski
changed the title
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## SNOW-2912540-decouple-oob-telemetry #4313 +/- ##
=======================================================================
- Coverage 95.23% 95.21% -0.03%
=======================================================================
Files 171 172 +1
Lines 44758 44771 +13
Branches 7678 7682 +4
=======================================================================
+ Hits 42627 42629 +2
- Misses 1344 1351 +7
- Partials 787 791 +4 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
sfc-gh-fpawlowski
force-pushed
the
SNOW-2912540-decouple-oob-telemetry
branch
from
August 20, 2026 16:28
c202a6b to
069c7ff
Compare
sfc-gh-fpawlowski
force-pushed
the
SNOW-2912540-secret-detector-import
branch
from
August 20, 2026 16:28
69382c5 to
07f8f83
Compare
sfc-gh-fpawlowski
force-pushed
the
SNOW-2912540-secret-detector-import
branch
from
August 20, 2026 17:13
07f8f83 to
cd9364b
Compare
sfc-gh-fpawlowski
changed the title
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
force-pushed
the
graphite-base/4313
branch
from
August 21, 2026 08:31
4d5e471 to
c921b91
Compare
sfc-gh-fpawlowski
force-pushed
the
SNOW-2912540-secret-detector-import
branch
from
August 21, 2026 08:31
c3ee5c6 to
9912bb7
Compare
sfc-gh-fpawlowski
changed the base branch from
graphite-base/4313
to
SNOW-2912540-decouple-oob-telemetry
August 21, 2026 08:31
…ctor imports connector_version is already imported in utils.py; this one-liner exposes a boolean flag so callers can gate imports or behavior that differs between the legacy connector (v3/v4) and the Universal Driver (v5+). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…; drop connector.options imports snowflake.connector.options is a backward-compat shim in v5 (UD) that will eventually be removed. Define MissingOptionalDependency, MissingPandas, MissingPyarrow, ModuleLikeObject, pandas, pyarrow, installed_pandas, and installed_pyarrow directly in _internal/utils.py and redirect all thirteen source-file imports there. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The v5 (Universal Driver) public snowflake.connector.telemetry shim does not always expose TelemetryData.TRUE/.FALSE (present on _internal.telemetry in some UD builds), which made every telemetry-sending Snowpark test raise AttributeError: type object 'TelemetryData' has no attribute 'FALSE'. Gate the TelemetryData import on IS_V5_DRIVER: on v5 prefer _internal.telemetry and fall back to the public shim; on v4 keep the legacy public import. Co-authored-by: Cursor <cursoragent@cursor.com>
…edefinition; re-source from connector 04c82d2 assumed connector.options had a gap and redefined MissingOptionalDependency, MissingPandas, MissingPyarrow, ModuleLikeObject, pyarrow, and installed_pyarrow locally. connector.options already provides all of these except installed_pyarrow (verified against the actually-installed v4.7.2 connector, which only exposes pyarrow itself and couples its availability to pandas's import tuple, not a standalone name). Re-import the four names that do exist there, gated on IS_V5_DRIVER so this keeps working once UD's connector._common.extras lands, and derive installed_pyarrow locally via isinstance instead of maintaining an independent, unprecedented MissingPyarrow resolution. pandas/installed_pandas (produced by the pre-existing _pandas_importer(), unrelated to 04c82d2) are intentionally left untouched -- unifying those is a separate follow-up.
…rent shape UD PR #1151 was updated since cb9f590 landed: MissingPandas is deleted outright from _common/extras.py (BehaviorDifferences.yaml #66), not kept as a deprecated re-export. Its suggested replacement, MissingOptionalDependency("pandas"), only works on v5 -- the real v4 connector's MissingOptionalDependency defines no __init__ override, so it only supports the no-arg-subclass pattern. Import MissingPandas only where it's real (v4). Add _missing_pandas() to build the sentinel with whichever construction style the active driver generation supports, centralizing the branch in _internal/utils.py rather than spreading IS_V5_DRIVER awareness to callers. Fix _pandas_importer() and mock/_options.py, both of which referenced MissingPandas directly and would otherwise NameError/ ImportError under IS_V5_DRIVER=True. installed_pyarrow also moves to a direct v5 import: #1151 fixed it to check pyarrow independently instead of mirroring installed_pandas, so it's now correct to import there instead of re-deriving locally. v4 still doesn't export it at all, so the local isinstance derivation stays there.
…n v5 The previous try/except hedged between two locations, neither of which actually has TelemetryData.TRUE/.FALSE on the current UD main: the public snowflake.connector.telemetry shim's TelemetryData has no TRUE/FALSE at all, and _internal.telemetry currently has no TelemetryData class either. The fallback branch was silently reachable and silently wrong. UD PR #1106 (open, not draft) adds TelemetryData/TelemetryField to _internal/telemetry.py specifically to match Snowpark's exact usage (PCTelemetryData(message=..., timestamp=...), .TRUE/.FALSE) -- confirmed by reading its actual diff. Import from there unconditionally on v5, no try/except: both branches now import from one definite, verified location.
…as from connector _pandas_importer() predates this whole effort and duplicated resolution the connector already does correctly on both driver generations -- including the "relative imports without dots" DataFrame workaround, now folded into UD's own _common.extras.pandas (confirmed on the not-yet-merged UD PR #1151/#1152; v4's options.py already had it). Add pandas/installed_pandas to the existing IS_V5_DRIVER-gated import block and delete the local resolution entirely. Verified the workaround isn't needed on Snowpark's side by running the exact invocation style its comment called out (pytest with tests/unit/ as cwd) -- no failure, consistent with both driver generations now handling it internally.
….extras mock/_options.py's MissingNumpy/numpy try-except was functionally identical to _common/extras.py's own numpy resolution (confirmed: pure duplicate, no fix to merge, per UD PR #1152's investigation). Import numpy from _common.extras on v5; v4 keeps its own MissingNumpy class since v4's options.py has no numpy handling to delegate to. Does not touch the pandas try/except in this file -- Local Testing deliberately never resolves pyarrow, unlike every other pandas-resolution path in this codebase (commit #1628).
Removing the function left only one blank line before class TempObjectType; black requires two before a top-level class definition.
…block - F401: pandas is imported purely for other modules to re-import from here, so it's never referenced elsewhere in this file. Split into its own import with a noqa, rather than noqa-ing a name inside a multi-line parenthesized import (which flake8 attributes to the opening line, not the name's own line). - E402: the IS_V5_DRIVER conditional-import block and _missing_pandas() ended up sitting between the top-of-file imports and two later ones (Row, VERSION). Moved those two imports up to stay contiguous.
…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.
sfc-gh-fpawlowski
force-pushed
the
SNOW-2912540-secret-detector-import
branch
from
August 21, 2026 09:36
9912bb7 to
6568b8e
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
IS_V5_DRIVER: bool = connector_version[0] >= 5to_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_detectordoes not exist in v5 (UD) —SecretDetectorlives atsnowflake.connector._common.secret_detector. Fixed with anIS_V5_DRIVER-gated conditional import inmock/_telemetry.py.snowflake.connector.optionsalready providesMissingOptionalDependency,ModuleLikeObject, andpyarrowon both driver generations — no gap to fill. Re-import these from the connector, gated onIS_V5_DRIVER, instead of redefining them locally in_internal/utils.py.installed_pyarrowis now imported directly on v5 (UD PR [Local Testing] SNOW-904981 Support Column bitwise operations and unary minus expression #1151 fixed it to checkpyarrowindependently instead of mirroringinstalled_pandas) but still derived locally viaisinstanceon v4 (not exported there at all).MissingPandasis 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, perBehaviorDifferences.yamlFix 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 ownIS_V5_DRIVERbranch._internal/utils.py's pre-existing_pandas_importer()(a second, independent pandas-resolution attempt that predates this whole effort) in favor of sourcingpandas/installed_pandasfrom the sameIS_V5_DRIVER-gated import as the rest of these names. Dedupsmock/_options.py's numpy handling againstconnector._common.extras.numpyon v5 (confirmed pure duplicate; v4 keeps its ownMissingNumpysince v4'soptions.pyhas 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'sTelemetryClient/TelemetryFieldimport was completely unconditional, pointing at the top-levelsnowflake.connector.telemetrymodule — a real implementation on v4, but a"""BACKWARD COMPATIBILITY MODULE ONLY"""stub on v5. Gated onIS_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.pyno longer exists there at all, fully moved). This same commit also corrects this file'sTelemetryDataimport, which an earlier commit here pointed at_internal.telemetry— that class moved to_commontoo 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.pyimportedReauthenticationRequestunconditionally fromsnowflake.connector.network— that module doesn't exist in UD at all (legacy'serrors.py/network.pysplit is consolidated intoerrors.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) addsReauthenticationRequest(ProgrammingError)toerrors.pyand removesnetwork.pyoutright, naming Snowpark's import site explicitly as the target. Gated the import (and its unit test mock intest_server_connection.py, which had the same unconditional-import bug) onIS_V5_DRIVER._missing_pandas()(had zero coverage aftertest__pandas_importer()was deleted alongside_pandas_importer()itself).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 importedinstalled_pandasfromsnowflake.connector.options— that module is deleted outright in UD PR [Local Testing] SNOW-904981 Support Column bitwise operations and unary minus expression #1151 (confirmed:f61156c7ais an ancestor ofSNOW-2912540-extras-to-common's current tip).IS_V5_DRIVER-gated options/pandas/pyarrow block out of_internal/utils.pyinto a new, dedicated_internal/options.py— mirrors the connector's ownoptions.py/_common.extras1:1, and mirrorsmock/_options.py's existing role as the scoped equivalent for local testing. Updated all 17 consumers (includingtypes.py, whose multi-line import a naive single-line grep initially missed — caught by a regex-based sweep afterward).options.pycomputes its ownIS_V5_DRIVERrather than importing it fromutils.py, sinceutils.pyitself needs names back fromoptions.py(its modin-optional-dependency code) — importing in both directions would be circular.Still open: UD PRs #1151, #1152, #1106, #1133, and #1224 are unmerged. This PR's
IS_V5_DRIVER=Truepaths are written against their current source but unverifiable end-to-end until they merge.Test plan
LocalTestOOBTelemetryService.export_queue_to_string()still callsSecretDetector.mask_secretscorrectly with both v4 and v5 connectorsgrep -rn "_pandas_importer" src/ tests/→ nothing;grep -rn "MissingNumpy" src/→ only inmock/_options.py's v4 branch;grep -rn "connector\.network\b" src/ tests/unit/→ only the v4 branch of the gated imports;grep -rn "connector\.options" tests/integ/→ nothing left; zero remaining imports of the moved names from_internal.utils(regex-based multi-line sweep, not just single-line grep)black/flake8clean on all 20 touched filesIS_V5_DRIVER=Truepaths againstconnector._common.extras/_common.telemetry/errors.ReauthenticationRequestonce UD PR [Local Testing] SNOW-904981 Support Column bitwise operations and unary minus expression #1151/ [Local Testing] SNOW-850263 Support Dataframe case insensitive collect #1152/SNOW-946900: Add internal parameter in stored proc registration to allow forcing inline code #1106/SNOW-964034 Enable skipped multistmt tests for stored proc #1133/SNOW-1023214: Support date_part argument in last_day #1224 mergeChecklist
Stack (via Graphite)
🤖 Generated with Claude Code