SNOW-2912540: add IS_V5_DRIVER constant; use it for SecretDetector/TelemetryData imports and pandas/pyarrow/numpy resolution - #4313
Open
sfc-gh-fpawlowski wants to merge 15 commits into
Conversation
…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>
In v5 (Universal Driver), SecretDetector lives at snowflake.connector._common.secret_detector; the legacy top-level snowflake.connector.secret_detector module no longer exists. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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}
…; 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>
| 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?
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>
…D build The previous commit's IS_V5_DRIVER branch assumed the v5 (UD) build exposes SecretDetector at snowflake.connector._common.secret_detector. Verified against the actually-installed UD build (2026.0.0) that this module does not exist there — SecretDetector is only ever at the top-level snowflake.connector.secret_detector path, same as v4. Switch to a try/except fallback so it works against both the current UD build and a possible future build that drops the top-level compat shim, without hardcoding a version assumption that doesn't hold today. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…talled UD build" This reverts commit bad7438.
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. |
…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.
sfc-gh-fpawlowski
marked this pull request as draft
August 14, 2026 17:29
…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.
3 tasks
…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).
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
…secret-detector-import
sfc-gh-fpawlowski
marked this pull request as ready for review
August 17, 2026 18:34
Removing the function left only one blank line before class TempObjectType; black requires two before a top-level class definition.
sfc-gh-fpawlowski
changed the title
…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.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## SNOW-2912540-decouple-oob-telemetry #4313 +/- ##
========================================================================
- Coverage 80.99% 67.33% -13.67%
========================================================================
Files 170 170
Lines 44663 44666 +3
Branches 7677 7680 +3
========================================================================
- Hits 36174 30075 -6099
- Misses 6969 11874 +4905
- Partials 1520 2717 +1197 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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'sTelemetryDataimport previously hedged with a try/except between two locations, neither of which actually hasTelemetryData.TRUE/.FALSEon the real current UD source. Fixed to import unconditionally from_internal.telemetryon v5, matching UD PR SNOW-946900: Add internal parameter in stored proc registration to allow forcing inline code #1106 (open, not draft) which adds exactly that surface to match Snowpark's usage.Still open: UD PRs #1151, #1152, and #1106 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 branchblack/pyrightclean after folding in SNOW-2912540: add IS_V5_DRIVER constant for version-conditioned connector imports #4310 and SNOW-2912540: remove _pandas_importer(), dedup mock/_options.py numpy handling #4317 (both had left the file unformatted)IS_V5_DRIVER=Truepaths againstconnector._common.extras/_internal.telemetryonce 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 mergeChecklist
Stack (via Graphite)
🤖 Generated with Claude Code