Skip to content

SNOW-UD: Add inline UD test workflow and tox connector-swap support#4117

Open
sfc-gh-fpawlowski wants to merge 10 commits intomainfrom
ud-ci-workflow
Open

SNOW-UD: Add inline UD test workflow and tox connector-swap support#4117
sfc-gh-fpawlowski wants to merge 10 commits intomainfrom
ud-ci-workflow

Conversation

@sfc-gh-fpawlowski
Copy link
Copy Markdown

  • Add ud-inline-tests.yml (workflow_dispatch only) to run Snowpark tests against the universal-driver Python connector installed from git+https.
  • Extend tox_install_cmd.sh with ud_connector_path branch: installs all deps, removes snowflake-connector-python, installs UD in its place.
  • Add ud_connector_path to tox.ini passenv so the variable reaches the install script.

No behavior change for regular Snowpark CI (ud_connector_path unset falls through to existing snowflake_path / PyPI logic).

  1. Which Jira issue is this PR addressing? Make sure that there is an accompanying issue to your PR.

    Fixes SNOW-NNNNNNN

  2. Fill out the following pre-review checklist:

    • I am adding a new automated test(s) to verify correctness of my new code
      • If this test skips Local Testing mode, I'm requesting review from @snowflakedb/local-testing
    • I am adding new logging messages
    • I am adding a new telemetry message
    • I am adding new credentials
    • I am adding a new dependency
    • If this is a new feature/behavior, I'm adding the Local Testing parity changes.
    • I acknowledge that I have ensured my changes to be thread-safe. Follow the link for more information: Thread-safe Developer Guidelines
    • 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. Follow the link for more information: AST Support Guidelines
  3. Please describe how your code solves the related issue.

    Please write a short description of how your code change solves the related issue.

@sfc-gh-fpawlowski sfc-gh-fpawlowski self-assigned this Mar 12, 2026
@sfc-gh-fpawlowski sfc-gh-fpawlowski added the NO-CHANGELOG-UPDATES This pull request does not need to update CHANGELOG.md label Mar 12, 2026
Comment thread .github/workflows/ud-inline-tests.yml
Comment thread .github/workflows/ud-inline-tests.yml Outdated
python-version:
description: 'Python version'
required: false
default: '3.10'
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.

is it only running one version at a time? should we consider default to the latest available python version in snowflake (3.13/3.14)?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Yes, for now we need it mostly to detect the API drift between both products. When we have solved all the missing contract elements, we can consider a full testing matrix.

And nice catch - changed to 3.13 : )

sfc-gh-fpawlowski and others added 5 commits March 13, 2026 06:40
- Add ud-inline-tests.yml (workflow_dispatch only) to run Snowpark
  tests against the universal-driver Python connector installed from
  git+https.
- Add scripts/ud_tox_install_cmd.sh — wrapper that delegates to the
  standard tox_install_cmd.sh, then swaps snowflake-connector-python
  for the UD connector. Original install script is untouched.
- Add [testenv:ud] and [testenv:ud-datasource] to tox.ini with their
  own install_command, passenv, and setenv. No changes to [testenv].

No behavior change for regular Snowpark CI — UD install logic is
fully isolated in the new wrapper script and testenv sections.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
TODO: remove pull_request trigger once all tests are passing.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Move the Universal Driver connector swap logic into the shared
tox_install_cmd.sh (conditional on ud_connector_path) so the UD
workflow runs the same tox environments as normal CI instead of
dedicated ud/ud-datasource envs.

- Add UD swap block to scripts/tox_install_cmd.sh (no-op when unset)
- Add ud_connector_path to [testenv] passenv
- Remove [testenv:ud] and [testenv:ud-datasource] from tox.ini
- Delete scripts/ud_tox_install_cmd.sh
- Workflow now invokes py${VER}-notdoctest-ci and datasource directly

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace the appended swap block with a proper 3-way conditional:
ud_connector_path → snowflake_path → PyPI (in priority order).

The appended approach ran the full PyPI install then the swap as a
separate pass on every tox install_command invocation. The integrated
approach mirrors the original design: UD takes explicit priority,
snowflake_path is only used when UD is not set, and a single logical
path runs per invocation.

Also removes the decorative comment banners added by the previous
edit, which were inconsistent with the surrounding code style.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add 'Run doctest' step using py${PYTHON_VERSION}-doctest-notudf-ci,
  matching the pattern from daily_precommit.yml.
- Change default python-version input from 3.10 to 3.13 (latest
  Snowflake-supported Python).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Comment on lines +77 to +78
curl https://packages.microsoft.com/keys/microsoft.asc | sudo apt-key add -
curl https://packages.microsoft.com/config/ubuntu/$(lsb_release -rs)/prod.list \
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.

Is there a safer way to install these than curling from an external URL?

@github-actions
Copy link
Copy Markdown


Thank you for your submission, we really appreciate it. Like many open-source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution. You can sign the CLA by just posting a Pull Request Comment same as the below format.


I have read the CLA Document and I hereby sign the CLA


You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot.

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Apr 22, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.41%. Comparing base (0aeccd9) to head (a01f5d6).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4117   +/-   ##
=======================================
  Coverage   95.41%   95.41%           
=======================================
  Files         171      171           
  Lines       43786    43786           
  Branches     7502     7502           
=======================================
  Hits        41778    41778           
  Misses       1227     1227           
  Partials      781      781           

☔ View full report in Codecov by Sentry.
📢 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.

PYTHON_VERSION: ${{ inputs.python-version || '3.13' }}
CLOUD_PROVIDER: ${{ inputs.cloud-provider || 'aws' }}
UD_BRANCH: ${{ inputs.ud-branch || 'snowpark-compatibility' }}
EXTRA_PYTEST_ADDOPTS: ${{ inputs.pytest-addopts || '-v --tb=long' }}
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.

The default value for EXTRA_PYTEST_ADDOPTS is '-v --tb=long', not an empty string. This causes the conditionals at lines 99 and 109 (if: ${{ env.EXTRA_PYTEST_ADDOPTS == '' }}) to always evaluate to false when using default inputs, skipping the doctest and datasource tests unexpectedly.

Impact: When the workflow is triggered without specifying pytest-addopts, only integration tests run. Doctest and datasource tests are always skipped.

Fix: Change the default to empty string:

EXTRA_PYTEST_ADDOPTS: ${{ inputs.pytest-addopts || '' }}

Or update the conditionals to check for the specific default value, though an empty default is cleaner.

Suggested change
EXTRA_PYTEST_ADDOPTS: ${{ inputs.pytest-addopts || '-v --tb=long' }}
EXTRA_PYTEST_ADDOPTS: ${{ inputs.pytest-addopts || '' }}

Spotted by Graphite

Fix in Graphite


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

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

Labels

NO-CHANGELOG-UPDATES This pull request does not need to update CHANGELOG.md

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants