diff --git a/.github/actions/restore-deps/action.yml b/.github/actions/restore-deps/action.yml new file mode 100644 index 000000000..1acd03225 --- /dev/null +++ b/.github/actions/restore-deps/action.yml @@ -0,0 +1,53 @@ +name: Restore Prewarmed Dependencies +description: | + Restores a fully-built in-project .venv from the "forkvenv-*" cache written by + warm-deps-cache.yml, so FORK PRs run offline without touching JFrog (#831). + + Outputs cache-hit. When true, the caller must SKIP setup-poetry (the .venv is + already complete) and only install Poetry itself. When false — a same-repo PR, + or a fork whose lockfiles changed and haven't been warmed — the caller falls + back to setup-poetry (JFrog OIDC), which works in every trusted context and on + cache-miss for same-repo PRs. + +inputs: + python-version: + description: Python version this leg targets (must match the warmed leg) + required: true + dependency-version: + description: 'Dependency set: "default" or "min"' + required: false + default: "default" + extras: + description: 'Extras installed in the warmed venv: "" (base), "pyarrow", or "kernel"' + required: false + default: "" + +outputs: + cache-hit: + description: "true if a warmed .venv was restored (install can be skipped)" + value: ${{ steps.restore.outputs.cache-matched-key != '' }} + +runs: + using: composite + steps: + - name: Restore warmed .venv + id: restore + uses: actions/cache/restore@0057852bfaa89a56745cba8c7296529d2fc39830 # v4 + with: + path: .venv + # The warmer appends a timestamp to the key (caches are immutable), so + # there is no fixed exact key to hit. restore-keys does a PREFIX match + # and returns the most-recently-created entry for this lockfile + + # matrix leg — exactly the freshest warmed venv. + key: forkvenv-${{ runner.os }}-${{ inputs.python-version }}-${{ inputs.dependency-version }}-${{ inputs.extras || 'base' }}-${{ hashFiles('**/poetry.lock') }}-never + restore-keys: | + forkvenv-${{ runner.os }}-${{ inputs.python-version }}-${{ inputs.dependency-version }}-${{ inputs.extras || 'base' }}-${{ hashFiles('**/poetry.lock') }}- + + - name: Report restore outcome + shell: bash + run: | + if [ -n "${{ steps.restore.outputs.cache-matched-key }}" ]; then + echo "Restored warmed .venv (${{ steps.restore.outputs.cache-matched-key }}) — install will be skipped." + else + echo "No warmed .venv for this lockfile/leg — caller falls back to setup-poetry (JFrog)." + fi diff --git a/.github/workflows/code-quality-checks.yml b/.github/workflows/code-quality-checks.yml index bb4db5b83..9cb2cbcc1 100644 --- a/.github/workflows/code-quality-checks.yml +++ b/.github/workflows/code-quality-checks.yml @@ -2,6 +2,27 @@ name: Code Quality Checks on: [pull_request] +# How fork PRs are made to work (issue #831) +# ------------------------------------------ +# Every job below installs deps via .github/actions/setup-poetry -> +# setup-jfrog, which needs a GitHub OIDC token to authenticate to the JFrog +# PyPI proxy. GitHub withholds that token from fork `pull_request` runs, and +# public PyPI is unreachable from the protected runners — so on a fork PR the +# JFrog path cannot run at all (this is what made every fork check red). +# +# Each job therefore takes a cache-first path: +# 1. restore-deps restores a fully-built .venv prewarmed by +# warm-deps-cache.yml (a trusted workflow that DOES have OIDC). +# 2. On a cache HIT (the normal fork path) we run the tools directly from +# .venv/bin — NOT `poetry run` — because even bootstrapping Poetry +# (`pip install poetry`) would hit JFrog. The restored venv already +# contains pytest/black/mypy and the connector, so no install is needed. +# 3. On a cache MISS (every same-repo PR, or a fork whose lockfiles changed +# before a maintainer re-warmed) we fall back to the original +# setup-poetry (JFrog OIDC) flow, unchanged. Same-repo PRs are unaffected. +# +# Each step branches on the restore step's output, `steps.deps.outputs.cache-hit`. + permissions: contents: read id-token: write @@ -26,28 +47,49 @@ jobs: steps: - name: Check out repository uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 + + # --- Fork path: restore a prewarmed .venv and run offline ------------- + - name: Restore prewarmed dependencies + id: deps + uses: ./.github/actions/restore-deps + with: + python-version: ${{ matrix.python-version }} + dependency-version: ${{ matrix.dependency-version }} + - name: Set up Python (cache hit — needed for the restored venv interpreter) + if: steps.deps.outputs.cache-hit == 'true' + uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5 + with: + python-version: ${{ matrix.python-version }} + - name: Run tests (offline, from prewarmed venv) + if: steps.deps.outputs.cache-hit == 'true' + run: .venv/bin/python -m pytest tests/unit -m "not realkernel" + + # --- Fallback path: same-repo PRs / cache miss use JFrog (unchanged) -- - name: Setup Poetry + if: steps.deps.outputs.cache-hit != 'true' uses: ./.github/actions/setup-poetry with: python-version: ${{ matrix.python-version }} cache-suffix: "${{ matrix.dependency-version }}-" - name: Install Python tools for custom versions - if: matrix.dependency-version != 'default' + if: steps.deps.outputs.cache-hit != 'true' && matrix.dependency-version != 'default' run: poetry run pip install toml packaging - name: Generate requirements file - if: matrix.dependency-version != 'default' + if: steps.deps.outputs.cache-hit != 'true' && matrix.dependency-version != 'default' run: | poetry run python scripts/dependency_manager.py ${{ matrix.dependency-version }} --output requirements-${{ matrix.dependency-version }}.txt echo "Generated requirements for ${{ matrix.dependency-version }} versions:" cat requirements-${{ matrix.dependency-version }}.txt - name: Override with custom dependency versions - if: matrix.dependency-version != 'default' + if: steps.deps.outputs.cache-hit != 'true' && matrix.dependency-version != 'default' run: poetry run pip install -r requirements-${{ matrix.dependency-version }}.txt - name: Show installed versions + if: steps.deps.outputs.cache-hit != 'true' run: | echo "=== Dependency Version: ${{ matrix.dependency-version }} ===" poetry run pip list - name: Run tests + if: steps.deps.outputs.cache-hit != 'true' run: poetry run python -m pytest tests/unit -m "not realkernel" run-unit-tests-with-arrow: @@ -69,11 +111,33 @@ jobs: steps: - name: Check out repository uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 + # libkrb5 is a runtime system lib (not in the venv), so install it on + # both the fork and fallback paths. - name: Install Kerberos system dependencies run: | sudo apt-get update sudo apt-get install -y libkrb5-dev + + # --- Fork path: restore prewarmed pyarrow venv, run offline ------------ + - name: Restore prewarmed dependencies + id: deps + uses: ./.github/actions/restore-deps + with: + python-version: ${{ matrix.python-version }} + dependency-version: ${{ matrix.dependency-version }} + extras: pyarrow + - name: Set up Python (cache hit — needed for the restored venv interpreter) + if: steps.deps.outputs.cache-hit == 'true' + uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5 + with: + python-version: ${{ matrix.python-version }} + - name: Run tests (offline, from prewarmed venv) + if: steps.deps.outputs.cache-hit == 'true' + run: .venv/bin/python -m pytest tests/unit -m "not realkernel" + + # --- Fallback path: same-repo PRs / cache miss use JFrog (unchanged) --- - name: Setup Poetry + if: steps.deps.outputs.cache-hit != 'true' uses: ./.github/actions/setup-poetry with: python-version: ${{ matrix.python-version }} @@ -84,22 +148,24 @@ jobs: install-args: "--extras pyarrow" cache-suffix: "pyarrow-${{ matrix.dependency-version }}-" - name: Install Python tools for custom versions - if: matrix.dependency-version != 'default' + if: steps.deps.outputs.cache-hit != 'true' && matrix.dependency-version != 'default' run: poetry run pip install toml packaging - name: Generate requirements file with pyarrow - if: matrix.dependency-version != 'default' + if: steps.deps.outputs.cache-hit != 'true' && matrix.dependency-version != 'default' run: | poetry run python scripts/dependency_manager.py ${{ matrix.dependency-version }} --output requirements-${{ matrix.dependency-version }}-arrow.txt echo "Generated requirements for ${{ matrix.dependency-version }} versions with PyArrow:" cat requirements-${{ matrix.dependency-version }}-arrow.txt - name: Override with custom dependency versions - if: matrix.dependency-version != 'default' + if: steps.deps.outputs.cache-hit != 'true' && matrix.dependency-version != 'default' run: poetry run pip install -r requirements-${{ matrix.dependency-version }}-arrow.txt - name: Show installed versions + if: steps.deps.outputs.cache-hit != 'true' run: | echo "=== Dependency Version: ${{ matrix.dependency-version }} with PyArrow ===" poetry run pip list - name: Run tests + if: steps.deps.outputs.cache-hit != 'true' run: poetry run python -m pytest tests/unit -m "not realkernel" run-unit-tests-with-kernel: @@ -113,6 +179,8 @@ jobs: python-version: ["3.10", "3.11", "3.12", "3.13", "3.14"] name: "Unit Tests + Kernel (Python ${{ matrix.python-version }})" + # base restore-deps entries omit 3.9-kernel (no kernel wheel there); this + # matrix already starts at 3.10, so every leg has a warmed counterpart. steps: - name: Check out repository @@ -121,7 +189,35 @@ jobs: run: | sudo apt-get update sudo apt-get install -y libkrb5-dev + + # --- Fork path: restore prewarmed kernel venv, run offline ------------ + # NOTE: the [kernel] extra pulls databricks-sql-kernel, the one PRIVATE + # (JFrog-only) package. Baking it into the warmed venv is precisely what + # lets a fork exercise the kernel routing without any JFrog credential. + - name: Restore prewarmed dependencies + id: deps + uses: ./.github/actions/restore-deps + with: + python-version: ${{ matrix.python-version }} + extras: kernel + - name: Set up Python (cache hit — needed for the restored venv interpreter) + if: steps.deps.outputs.cache-hit == 'true' + uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5 + with: + python-version: ${{ matrix.python-version }} + - name: Assert the real kernel wheel is installed (offline) + if: steps.deps.outputs.cache-hit == 'true' + run: .venv/bin/python -c "import databricks_sql_kernel as k; assert k.__file__, 'kernel wheel missing __file__ — not the real wheel'; print('real kernel wheel:', k.__file__)" + - name: Unit tests, realkernel deselected (offline) + if: steps.deps.outputs.cache-hit == 'true' + run: .venv/bin/python -m pytest tests/unit -m "not realkernel" + - name: Drive use_kernel=True through the REAL wheel — routing (offline) + if: steps.deps.outputs.cache-hit == 'true' + run: .venv/bin/python -m pytest tests/unit/test_session.py -m realkernel -v + + # --- Fallback path: same-repo PRs / cache miss use JFrog (unchanged) -- - name: Setup Poetry + if: steps.deps.outputs.cache-hit != 'true' uses: ./.github/actions/setup-poetry with: python-version: ${{ matrix.python-version }} @@ -132,19 +228,23 @@ jobs: install-args: "--extras kernel" cache-suffix: "kernel-" - name: Show installed versions + if: steps.deps.outputs.cache-hit != 'true' run: | echo "=== with databricks-sql-kernel ===" poetry run pip list - name: Assert the real kernel wheel is installed (not a stub) + if: steps.deps.outputs.cache-hit != 'true' run: | poetry run python -c "import databricks_sql_kernel as k; assert k.__file__, 'kernel wheel missing __file__ — not the real wheel'; print('real kernel wheel:', k.__file__)" - name: Unit tests (kernel wheel present, realkernel deselected) + if: steps.deps.outputs.cache-hit != 'true' # The bulk of tests/unit fakes databricks_sql_kernel in # sys.modules, so the real-wheel routing test is deselected here # and run on its own below (a shared session would shadow the # real wheel — both real-wheel tests fail loudly if that happens). run: poetry run python -m pytest tests/unit -m "not realkernel" - name: Drive use_kernel=True through the REAL wheel (routing) + if: steps.deps.outputs.cache-hit != 'true' # Separate invocation, explicit file path: never collects the # fake-module test file, so sys.modules stays unpolluted. This is # the no-network proof that sql.connect(use_kernel=True) actually @@ -162,11 +262,26 @@ jobs: steps: - name: Check out repository uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 + - name: Restore prewarmed dependencies + id: deps + uses: ./.github/actions/restore-deps + with: + python-version: ${{ matrix.python-version }} + - name: Set up Python (cache hit — needed for the restored venv interpreter) + if: steps.deps.outputs.cache-hit == 'true' + uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5 + with: + python-version: ${{ matrix.python-version }} + - name: Black (offline, from prewarmed venv) + if: steps.deps.outputs.cache-hit == 'true' + run: .venv/bin/black --check src - name: Setup Poetry + if: steps.deps.outputs.cache-hit != 'true' uses: ./.github/actions/setup-poetry with: python-version: ${{ matrix.python-version }} - name: Black + if: steps.deps.outputs.cache-hit != 'true' run: poetry run black --check src check-types: @@ -179,11 +294,30 @@ jobs: steps: - name: Check out repository uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 + - name: Restore prewarmed dependencies + id: deps + uses: ./.github/actions/restore-deps + with: + python-version: ${{ matrix.python-version }} + - name: Set up Python (cache hit — needed for the restored venv interpreter) + if: steps.deps.outputs.cache-hit == 'true' + uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5 + with: + python-version: ${{ matrix.python-version }} + - name: Mypy (offline, from prewarmed venv) + if: steps.deps.outputs.cache-hit == 'true' + # --install-types would reach the index; the warmed venv already has + # the stub packages, so run mypy without it on the offline path. + run: | + mkdir -p .mypy_cache + .venv/bin/mypy --non-interactive src - name: Setup Poetry + if: steps.deps.outputs.cache-hit != 'true' uses: ./.github/actions/setup-poetry with: python-version: ${{ matrix.python-version }} - name: Mypy + if: steps.deps.outputs.cache-hit != 'true' run: | mkdir .mypy_cache poetry run mypy --install-types --non-interactive src diff --git a/.github/workflows/warm-deps-cache.yml b/.github/workflows/warm-deps-cache.yml new file mode 100644 index 000000000..c029ff020 --- /dev/null +++ b/.github/workflows/warm-deps-cache.yml @@ -0,0 +1,198 @@ +name: Warm Dependency Cache + +# Sole writer of the "forkvenv-*" GitHub Actions cache that lets FORK PRs run +# unit / lint / type CI fully offline (issue #831). +# +# Why this exists +# --------------- +# Every job in code-quality-checks.yml installs dependencies through +# .github/actions/setup-poetry -> setup-jfrog, which mints a JFrog PyPI token +# via GitHub OIDC. GitHub deliberately withholds the OIDC token +# (ACTIONS_ID_TOKEN_REQUEST_TOKEN) from `pull_request` runs originating in a +# fork, so setup-jfrog dies with "unbound variable" and EVERY required check is +# red on every fork PR — regardless of the diff (see #831, reported against the +# trivial fork PR #671). +# +# Public PyPI is not reachable from the protected runners (everything must go +# through the JFrog proxy), and a fork run can obtain no JFrog credential at +# all. The fix (Databricks SOP, mirrors databricks/dbt-databricks) is to invert +# the flow: a TRUSTED workflow here — which does get OIDC — pre-builds the +# in-project .venv for every matrix leg the PR jobs consume and saves it to the +# Actions cache. The fork PR then RESTORES that .venv and skips `poetry install` +# entirely, so it never needs JFrog. Real tests still run; the fork just holds +# no credentials, so there is no pull_request_target / secret-exposure risk. +# +# What it caches +# -------------- +# The fully-built in-project .venv, one cache entry per +# (python-version x dependency-version x extras) +# leg — the same axes code-quality-checks.yml iterates. The min / pyarrow / +# kernel variants pip-install overrides ON TOP of `poetry install`, so the venv +# is saved AFTER those steps to capture the complete resolved environment. +# +# Cache key: forkvenv------ +# GitHub caches are immutable and evict after 7 days of no reads; the timestamp +# suffix lets a re-warm write a fresh entry (old ones age out), and the daily +# schedule keeps the cache from expiring. Consumers restore by the +# "...--" prefix via restore-keys, so they pick up the newest entry +# for the current lockfile automatically. +# +# Fork dependency changes +# ----------------------- +# If a fork PR modifies poetry.lock / pyproject.toml, its lockhash won't match +# any warmed entry and its CI will miss the cache. A maintainer then reviews the +# dependency change and runs this workflow with `pr_number = N`: it fetches ONLY +# the fork's lockfiles (git checkout FETCH_HEAD -- , no source code) +# and warms a cache entry for the new hash. The contributor re-runs CI. + +on: + push: + branches: [main] + paths: + - "poetry.lock" + - "pyproject.toml" + schedule: + # Daily at 06:07 UTC — beats GitHub's 7-day cache eviction. Off the :00 + # mark to avoid the cron thundering herd. + - cron: "7 6 * * *" + workflow_dispatch: + inputs: + pr_number: + description: "Fork PR number to warm the cache for (reads lockfiles from the fork branch). Leave empty to warm from main." + required: false + type: string + +permissions: + contents: read + id-token: write + pull-requests: read + +# Only one warm run at a time per ref — a schedule tick and a push shouldn't +# race to write overlapping cache entries. +concurrency: + group: warm-deps-${{ github.ref }} + cancel-in-progress: false + +jobs: + warm: + runs-on: + group: databricks-protected-runner-group + labels: linux-ubuntu-latest + strategy: + # One leg failing (e.g. a transient JFrog hiccup) shouldn't starve the + # rest of the cache — warm everything we can. + fail-fast: false + matrix: + # Mirror code-quality-checks.yml exactly so every fork check has a + # matching warmed entry. extras: "" is the base unit-test / lint / type + # environment; pyarrow and kernel back the two extra unit-test tiers. + python-version: ["3.9", "3.10", "3.11", "3.12", "3.13", "3.14"] + dependency-version: ["default", "min"] + extras: ["", "pyarrow", "kernel"] + exclude: + # min deps are only exercised on 3.9/3.10/3.11 in the PR matrix. + - python-version: "3.12" + dependency-version: "min" + - python-version: "3.13" + dependency-version: "min" + - python-version: "3.14" + dependency-version: "min" + # The kernel wheel is cp310-abi3 (Requires-Python >=3.10); the + # [kernel] extra is a no-op on 3.9, so there's no kernel leg to warm. + - python-version: "3.9" + extras: "kernel" + + name: "Warm (py ${{ matrix.python-version }}, ${{ matrix.dependency-version }} deps, extras=${{ matrix.extras || 'base' }})" + + steps: + - name: Check out repository + uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 + with: + fetch-depth: 0 + + - name: Overlay fork PR lockfiles + if: inputs.pr_number != '' + env: + GH_TOKEN: ${{ github.token }} + PR_NUMBER: ${{ inputs.pr_number }} + run: | + set -euo pipefail + # Fetch ONLY the fork's dependency files — never its source or + # workflows — so warming a fork's cache can't execute fork-controlled + # code in this trusted, OIDC-bearing context. + PR_DATA=$(gh api "repos/${GITHUB_REPOSITORY}/pulls/${PR_NUMBER}") + FORK_REPO=$(echo "$PR_DATA" | jq -r '.head.repo.full_name') + FORK_REF=$(echo "$PR_DATA" | jq -r '.head.ref') + echo "Warming cache for PR #${PR_NUMBER} from ${FORK_REPO}@${FORK_REF}" + git remote add fork "https://github.com/${FORK_REPO}.git" + git fetch --depth=1 fork "${FORK_REF}" + git checkout FETCH_HEAD -- poetry.lock pyproject.toml + echo "Overlaid poetry.lock / pyproject.toml from the fork branch." + + - name: Install Kerberos system dependencies + if: matrix.extras == 'pyarrow' || matrix.extras == 'kernel' + run: | + sudo apt-get update + sudo apt-get install -y libkrb5-dev + + - name: Setup Poetry (builds .venv from JFrog) + uses: ./.github/actions/setup-poetry + with: + python-version: ${{ matrix.python-version }} + # extras="" -> base install (no --extras). Otherwise install exactly + # the one extra this leg targets, matching the PR job's isolation. + install-args: ${{ matrix.extras == '' && '' || format('--extras {0}', matrix.extras) }} + # Distinct suffix per leg so setup-poetry's own "venv-*" cache (used by + # same-repo PRs) never collides across dependency-version / extras. + cache-suffix: "warm-${{ matrix.dependency-version }}-${{ matrix.extras || 'base' }}-" + + - name: Install Python tools for custom versions + if: matrix.dependency-version != 'default' + run: poetry run pip install toml packaging + + - name: Generate + apply min dependency versions + if: matrix.dependency-version != 'default' + run: | + set -euo pipefail + poetry run python scripts/dependency_manager.py ${{ matrix.dependency-version }} \ + --output requirements-${{ matrix.dependency-version }}.txt + echo "Generated requirements for ${{ matrix.dependency-version }} versions:" + cat requirements-${{ matrix.dependency-version }}.txt + # pip-installs the pinned floor ON TOP of the poetry venv — the venv is + # cached AFTER this so the fork restores the complete resolved env. + poetry run pip install -r requirements-${{ matrix.dependency-version }}.txt + + - name: Prime mypy stub packages into the venv + # check-types runs `mypy --install-types`, which fetches stub packages + # from the index at runtime. Bake them into the warmed venv now (on the + # default/base leg the type job restores) so the offline fork run has + # them and can drop --install-types. + if: matrix.extras == '' && matrix.dependency-version == 'default' + run: | + mkdir -p .mypy_cache + poetry run mypy --install-types --non-interactive src || true + + - name: Show installed versions + run: | + echo "=== py ${{ matrix.python-version }} | ${{ matrix.dependency-version }} deps | extras=${{ matrix.extras || 'base' }} ===" + poetry run pip list + + - name: Compute cache key + id: key + run: | + set -euo pipefail + TIMESTAMP=$(date -u +%Y%m%d%H%M%S) + LOCK_HASH="${{ hashFiles('**/poetry.lock') }}" + PY="${{ matrix.python-version }}" + DEP="${{ matrix.dependency-version }}" + EXTRAS="${{ matrix.extras || 'base' }}" + BASE="forkvenv-${RUNNER_OS}-${PY}-${DEP}-${EXTRAS}-${LOCK_HASH}" + echo "base=${BASE}" >> "$GITHUB_OUTPUT" + echo "full=${BASE}-${TIMESTAMP}" >> "$GITHUB_OUTPUT" + echo "Cache key: ${BASE}-${TIMESTAMP}" + + - name: Save .venv to fork cache + uses: actions/cache/save@0057852bfaa89a56745cba8c7296529d2fc39830 # v4 + with: + path: .venv + key: ${{ steps.key.outputs.full }}