Skip to content

Navigation Menu

Sign in
Sign up

Switch UD reporting to 5-group test matrix, fix concurrency + verification - #4329

Draft
sfc-gh-fpawlowski wants to merge 36 commits into
SNOW-2912540-write-pandas-raw-msg from
SNOW-2912540-ud-reporting
Draft

Switch UD reporting to 5-group test matrix, fix concurrency + verification #4329
sfc-gh-fpawlowski wants to merge 36 commits into
SNOW-2912540-write-pandas-raw-msg from
SNOW-2912540-ud-reporting

Conversation

@sfc-gh-fpawlowski

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

Copy link
Copy Markdown
Collaborator

What

Replaces this branch's 2-group UD test matrix (integration/scala, ported from ud-ci-workflow-auto-triggered-full) with the 5-group architecture (unit-integ/scala/modin/datasource/doctest, ported from ud-ci-workflow-auto-triggered-full-improvements): UD is built once via a dedicated build-ud job (wheel from snowflakedb/universal-driver) and consumed as a downloaded artifact, instead of each test job pip-installing a git ref inline.

Two deliberate deviations from ud-ci-workflow-auto-triggered-full-improvements

  1. Concurrency block removed. The source branch's concurrency: group: ud-tests-${{ github.ref }}, cancel-in-progress: true keys only on the git ref hosting the workflow file, not on the UD ref under test. odbc-reports dispatches this same ref repeatedly, concurrently, with a different ud-ref per historical snapshot date — that keying would cancel one in-flight backfill run whenever another started, and a cancelled run reports as a permanent errors: 1 rather than a retriable missing entry. Confirmed none of odbc-reports' other four UD-dispatch sibling jobs (snowflake-cli, snowflake-sqlalchemy, dbt-adapters, airflow) use a concurrency block either, so this matches the existing convention rather than inventing a one-off.

  2. Fail-fast UD-install verification redesigned to poison output, not just exit. This job has continue-on-error: true at the job level plus a separate if: always() "Extract results" step — a bare exit 1 wouldn't stop a silently-installed legacy connector's misleading pass/fail counts from being scraped and reported as real UD results. The verification step now overwrites reports/test-output.log with 1 errors on failure so the downstream extraction reflects the truth.

scripts/tox_install_cmd.sh needed no changes (already hardened identically on both source branches: set -o pipefail, --no-deps on the UD reinstall). tox.ini copied verbatim from ud-ci-workflow-auto-triggered-full-improvements (already passes through ud_connector_path/UD_RERUN_FLAGS/JUNIT_REPORT_DIR).

Scope

No src/ or tests/ changes — workflow/CI files only.

Context

This is a long-lived CI-reference branch (same lifecycle as ud-ci-workflow-auto-triggered-full-improvements), not intended to merge into main — it's what snowflake-eng/odbc-reports' update-python-tests-cache.yml dispatches against to track Snowpark-on-UD pass rate over time. This PR is for reviewability of the diff.

sfc-gh-fpawlowski and others added 30 commits August 21, 2026 08:22
...ncoding
The deprecated `create_temp_table` parameter was being emitted to the
proto AST as a separate boolean field even though the runtime already
translates it to `table_type="temporary"`. This meant the AST decoder
had to handle two representations for the same thing.
Fix: move the deprecation coercion before the AST block in save_as_table
so `table_type` is already resolved when emitted; remove the deprecated
field from both AST emission sites (WriteTable and WritePandas); update
the internal cache_result mock path to pass table_type="temp" directly;
mark the proto fields as deprecated.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
..._table removal
Remove create_temp_table from the expected encoded AST and unparser
output in the write_pandas golden test — the field is no longer emitted
to the proto since the deprecation coercion now happens before the AST
block, making table_type the sole carrier of this information.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
...ncoding
The deprecated `create_temp_table` parameter was being emitted to the
proto AST as a separate boolean field even though the runtime already
translates it to `table_type="temporary"`. This meant the AST decoder
had to handle two representations for the same thing.
Fix: move the deprecation coercion before the AST block in save_as_table
so `table_type` is already resolved when emitted; remove the deprecated
field from both AST emission sites (WriteTable and WritePandas); update
the internal cache_result mock path to pass table_type="temp" directly;
mark the proto fields as deprecated.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
These constants were previously imported from snowflake-connector-python.
The Universal Driver connector no longer owns them (it has no internal use
for either symbol), so define them locally to remove the coupling.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The Universal Driver connector's constants module won't provide
ENV_VAR_PARTNER either (per review discussion on
snowflake-eng/universal-driver#512), so define it locally in
server_connection.py alongside the other now-local backward
compatibility constants (ASYNC_RETRY_PATTERN, compat.OK).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
...into analyzer_utils
Removes the dependency on these private snowflake-connector-python functions
(previously imported from connector.pandas_tools). Ports the implementation
from universal-driver PR #518 directly into Snowpark so the connector no longer
needs to expose staging helpers as part of its public surface.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
...ned pandas staging helpers
black never ran on the ported code (4 blocks needed line-wrapping).
The two cursor.execute(..., _force_qmark_paramstyle=True) calls also
fail pyright: SnowflakeCursor.execute's two @overload stubs in
snowflake-connector-python never declare _force_qmark_paramstyle,
only the real implementation signature does. That gap was invisible
while this code lived in the connector package; porting it into
analyzer_utils.py exposes it to Snowpark's own pyright run.
...tings
_stage_sql (inlined from universal-driver PR #518 in the prior commit)
faithfully maps compression="gzip" to COMPRESSION=auto in the generated
CREATE...STAGE...FILE_FORMAT SQL, same as it already does for the COPY INTO
clause (see copy_compression in this test). The stage-creation assertion
still expected the literal "gzip", unlike its COPY INTO counterpart a few
lines below which already accounted for the auto mapping via
copy_compression. Update it to match.
...lemetry_oob
The UD's telemetry_oob.TelemetryService is a no-op stub (BD#45) that lacks
batch_size, causing an AttributeError at runtime. Make LocalTestOOBTelemetryService
standalone: add its own singleton, queue, batch_size, _enabled, and close().
Also fixes the pre-existing _enable typo (should have been _enabled).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The decoupling from connector.telemetry_oob ported add()/flush()/enable()/
disable() but missed size() (connector/telemetry_oob.py:542,
`return self.queue.qsize()`), which several tests
(tests/mock/test_oob_telemetry.py, tests/mock/test_multithreading.py) call
directly on the service instance.
mock/_telemetry.py imported snowflake.connector.secret_detector.SecretDetector
unconditionally -- the only reason UD PR #598 ports SecretDetector into the
Universal Driver at all is to satisfy this one Snowpark import (SecretDetector
is not otherwise used or maintained by UD). Per BehaviorDifferences.yaml #45,
OOB telemetry is disabled on the backend anyway, so this masking is
defense-in-depth on the outbound payload, not a response to active
exploitation -- but export_queue_to_string() feeds a real
requests.Session().post() to a live analytics endpoint, so it's not a safe
no-op either.
Ports only what _telemetry.py actually calls (mask_secrets -> masked string),
from UD's actual _common/secret_detector.py source (PR #598) rather than the
older, less complete real v4 connector's version -- verified UD's copy has
3 additional maskers (OAuth tokens, OAuth client secrets, passcodes) plus a
PASSWORD_PATTERN false-positive fix and a wire-format fix to
CONNECTION_TOKEN_PATTERN, all needed for correctness. Drops everything that
exists solely for UD's backward-compat contract: @backward_compatibility,
MaskedMessageData's 3-tuple return (only masked_text is ever read),
logging.Formatter inheritance, and the classmethod-vs-staticmethod workaround.
Fixes the exception-handling branch rather than porting it straight: the
legacy/UD code does masked_text = str(ex) on failure, which can leak
exception-reflected input. Returns a static sentinel instead -- this was
already flagged as a real finding by the security bot on UD #598.
After this, mock/_telemetry.py has zero remaining imports from
snowflake.connector.
Test coverage: ported every behavioral test from UD PR #598's
test_secret_detector.py TestMaskSecrets class (27 cases via pytest count),
adapted from the 3-tuple/class-method API to the plain-function API here,
plus one exception-handling test adapted to assert the static sentinel
instead of the leaked exception text. Dropped TestFormatter and the
logging.Formatter-specific exception tests -- not applicable, since
mask_secrets is a plain function here, not a logging.Formatter subclass.
Corrects the previous commit: this is now an exact copy of UD's
_common/secret_detector.py (drivers#598), not a collapsed rewrite. Keeps the
SecretDetector class, MaskedMessageData, the (is_masked, masked_text, err_str)
3-tuple return, classmethods, logging.Formatter inheritance, format(),
create_formatting_error_log(), and masked_text = str(ex) on failure --
unchanged, even though the str(ex) behavior is a known real finding (flagged
by the security bot on UD #598); fixing it is a separate call for whoever
reviews this, not bundled into a move.
Only removed what's physically impossible to keep: @backward_compatibility,
install_backward_compatibility_getattr and its import, and the trailing
install_backward_compatibility_getattr(__name__) call -- none of that
mechanism exists in snowpark-python. The mask_secrets() docstring explaining
why the maskers are classmethods (to survive @backward_compatibility
stashing the class out of module globals) is left as-is even though that
rationale no longer applies here -- not silently deleted or rewritten as
part of this move.
_telemetry.py's call site goes back to unpacking the 3-tuple unmodified;
test_secret_detector.py is now a full copy of UD's test suite (TestMaskSecrets,
TestMaskSecretsExceptionHandling, TestFormatter), not just the masking-behavior
subset -- format()/logging.Formatter are kept, so their tests are too.
...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.
The UD's SnowflakeCursor exposes the client-generated request UUID as
a public `request_id` property; `_request_id` is only a backward-compat
alias slated for removal. The legacy v4 connector is the opposite: it
only ever sets `self._request_id` as a plain instance attribute and has
no public `request_id` property. Branch on IS_V5_DRIVER in the one call
site that reads this (execute_and_notify_query_listener) and its unit
test mock.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Blocks pre-commit's insert-py-license hook (and therefore "Check linting",
which gates the entire test matrix) for every branch stacked on top of
this one.

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

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

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

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

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

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

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

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

Previous 5-group attempt on this branch accumulated a lot of ad-hoc
debugging hacks (checkout-target flip-flopping, dead-token additions,
manual git-fetch detours) while chasing what turned out to be a red
herring. Reverting those left the branch back at its original 2-group
form -- losing the intended 5-group scope in the process.
This rebuilds cleanly from ud-ci-workflow-auto-triggered-full-improvements
(unit-integ/scala/modin/datasource/doctest), using the already-proven
patch from snowpark-ud-job-testing (scripts/patch_new_workflow_for_verification.py
+ verify_ud_snippet.py) for the install-hardening + UD-verification logic,
instead of reinventing it. repository: snowflakedb/universal-driver is
left untouched from the source -- this is the repo snowpark-ud-job-testing
has reliably dispatched against for weeks (see its JOBS.md history), unlike
snowflakedb/drivers which had persistent, never-fully-root-caused
actions/checkout fetch failures earlier this session.
Only change beyond the source: dropped the concurrency block (confirmed
none of odbc-reports' other UD-dispatch sibling jobs use one either -- see
commit history for the concurrency-collision risk this avoids).

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

.tox/"$TOX"/bin/pip install --force-reinstall --no-deps "${ud_connector_path}"
echo "reinstall exit code: $?"
elif [ -n "${snowflake_path}" ]; then
.tox/"$TOX"/bin/pip install --force-reinstall --no-deps ${snowflake_path}/snowflake_connector_python*.whl

@graphite-app graphite-app Bot Aug 24, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing quotes around ${snowflake_path} variable. This will cause word splitting and glob expansion if the path contains spaces or special characters.

# Current (broken):
.tox/"$TOX"/bin/pip install --force-reinstall --no-deps ${snowflake_path}/snowflake_connector_python*.whl
# Fixed:
.tox/"$TOX"/bin/pip install --force-reinstall --no-deps "${snowflake_path}"/snowflake_connector_python*.whl

This is inconsistent with line 221 which correctly quotes "${ud_connector_path}".

Suggested change
.tox/"$TOX"/bin/pip install --force-reinstall --no-deps ${snowflake_path}/snowflake_connector_python*.whl
.tox/"$TOX"/bin/pip install --force-reinstall --no-deps "${snowflake_path}"/snowflake_connector_python*.whl

Spotted by Graphite

Fix in Graphite


Is this helpful? React 👍 or 👎 to let us know.

sfc-gh-fpawlowski marked this pull request as draft August 24, 2026 03:24
cursor Bot force-pushed the SNOW-2912540-write-pandas-raw-msg branch from 0c1595c to 4475ec9 Compare September 2, 2026 06:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@graphite-app graphite-app[bot] graphite-app[bot] left review comments
@snowflake-security-bot snowflake-security-bot[bot] snowflake-security-bot[bot] left review comments
@sfc-gh-yixie sfc-gh-yixie Awaiting requested review from sfc-gh-yixie sfc-gh-yixie is a code owner automatically assigned from snowflakedb/snowpark-python-api-reviewers
@sfc-gh-mayliu sfc-gh-mayliu Awaiting requested review from sfc-gh-mayliu sfc-gh-mayliu is a code owner automatically assigned from snowflakedb/snowpark-python-api-reviewers
@sfc-gh-bkogan sfc-gh-bkogan Awaiting requested review from sfc-gh-bkogan sfc-gh-bkogan is a code owner automatically assigned from snowflakedb/snowpark-python-api-reviewers

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

AltStyle によって変換されたページ (->オリジナル) /