Skip to content

Commit 8b6bf17

Browse files
wyattscarpenterwyattscarpenter
authored
Merge branch 'databricks:main' into patch-3
2 parents 48dde22 + 54e479c commit 8b6bf17

24 files changed

Lines changed: 2514 additions & 350 deletions

‎.github/actions/restore-deps/action.yml‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
name: Restore Prewarmed Dependencies
2+
description: |
3+
Restores a fully-built in-project .venv from the "forkvenv-*" cache written by
4+
warm-deps-cache.yml, so FORK PRs run offline without touching JFrog (#831).
5+
6+
Outputs cache-hit. When true, the caller must SKIP setup-poetry (the .venv is
7+
already complete) and only install Poetry itself. When false — a same-repo PR,
8+
or a fork whose lockfiles changed and haven't been warmed — the caller falls
9+
back to setup-poetry (JFrog OIDC), which works in every trusted context and on
10+
cache-miss for same-repo PRs.
11+
12+
inputs:
13+
python-version:
14+
description: Python version this leg targets (must match the warmed leg)
15+
required: true
16+
dependency-version:
17+
description: 'Dependency set: "default" or "min"'
18+
required: false
19+
default: "default"
20+
extras:
21+
description: 'Extras installed in the warmed venv: "" (base), "pyarrow", or "kernel"'
22+
required: false
23+
default: ""
24+
25+
outputs:
26+
cache-hit:
27+
description: "true if a warmed .venv was restored (install can be skipped)"
28+
value: ${{ steps.restore.outputs.cache-matched-key != '' }}
29+
30+
runs:
31+
using: composite
32+
steps:
33+
- name: Restore warmed .venv
34+
id: restore
35+
uses: actions/cache/restore@0057852bfaa89a56745cba8c7296529d2fc39830 # v4
36+
with:
37+
path: .venv
38+
# The warmer appends a timestamp to the key (caches are immutable), so
39+
# there is no fixed exact key to hit. restore-keys does a PREFIX match
40+
# and returns the most-recently-created entry for this lockfile +
41+
# matrix leg — exactly the freshest warmed venv.
42+
key: forkvenv-${{ runner.os }}-${{ inputs.python-version }}-${{ inputs.dependency-version }}-${{ inputs.extras || 'base' }}-${{ hashFiles('**/poetry.lock') }}-never
43+
restore-keys: |
44+
forkvenv-${{ runner.os }}-${{ inputs.python-version }}-${{ inputs.dependency-version }}-${{ inputs.extras || 'base' }}-${{ hashFiles('**/poetry.lock') }}-
45+
46+
- name: Report restore outcome
47+
shell: bash
48+
run: |
49+
if [ -n "${{ steps.restore.outputs.cache-matched-key }}" ]; then
50+
echo "Restored warmed .venv (${{ steps.restore.outputs.cache-matched-key }}) — install will be skipped."
51+
else
52+
echo "No warmed .venv for this lockfile/leg — caller falls back to setup-poetry (JFrog)."
53+
fi

‎.github/workflows/code-coverage.yml‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,8 +63,21 @@ jobs:
6363
install-args: "--all-extras"
6464
- name: Run all tests with coverage
6565
continue-on-error: false
66+
# This job installs --all-extras, so the REAL databricks-sql-kernel
67+
# wheel is present. The unit suite fakes databricks_sql_kernel in
68+
# sys.modules, which would shadow the real wheel in this shared
69+
# session — so the kernel-backed suites that need the real wheel
70+
# are excluded here and covered by the dedicated kernel-e2e.yml
71+
# (isolated session, real wheel, live warehouse):
72+
# - --ignore the kernel e2e files (they assert the real wheel and
73+
# now FAIL LOUDLY rather than silently skip if shadowed), and
74+
# - -m "not realkernel" deselects the no-network real-wheel
75+
# routing test for the same reason.
6676
run: |
6777
poetry run pytest tests/unit tests/e2e \
78+
--ignore=tests/e2e/test_kernel_backend.py \
79+
--ignore=tests/e2e/test_kernel_tls.py \
80+
-m "not realkernel" \
6881
-n 4 \
6982
--dist=loadgroup \
7083
--cov=src \

‎.github/workflows/code-quality-checks.yml‎

Lines changed: 197 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,27 @@ name: Code Quality Checks
22

33
on: [pull_request]
44

5+
# How fork PRs are made to work (issue #831)
6+
# ------------------------------------------
7+
# Every job below installs deps via .github/actions/setup-poetry ->
8+
# setup-jfrog, which needs a GitHub OIDC token to authenticate to the JFrog
9+
# PyPI proxy. GitHub withholds that token from fork `pull_request` runs, and
10+
# public PyPI is unreachable from the protected runners — so on a fork PR the
11+
# JFrog path cannot run at all (this is what made every fork check red).
12+
#
13+
# Each job therefore takes a cache-first path:
14+
# 1. restore-deps restores a fully-built .venv prewarmed by
15+
# warm-deps-cache.yml (a trusted workflow that DOES have OIDC).
16+
# 2. On a cache HIT (the normal fork path) we run the tools directly from
17+
# .venv/bin — NOT `poetry run` — because even bootstrapping Poetry
18+
# (`pip install poetry`) would hit JFrog. The restored venv already
19+
# contains pytest/black/mypy and the connector, so no install is needed.
20+
# 3. On a cache MISS (every same-repo PR, or a fork whose lockfiles changed
21+
# before a maintainer re-warmed) we fall back to the original
22+
# setup-poetry (JFrog OIDC) flow, unchanged. Same-repo PRs are unaffected.
23+
#
24+
# Each step branches on the restore step's output, `steps.deps.outputs.cache-hit`.
25+
526
permissions:
627
contents: read
728
id-token: write
@@ -26,29 +47,50 @@ jobs:
2647
steps:
2748
- name: Check out repository
2849
uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
50+
51+
# --- Fork path: restore a prewarmed .venv and run offline -------------
52+
- name: Restore prewarmed dependencies
53+
id: deps
54+
uses: ./.github/actions/restore-deps
55+
with:
56+
python-version: ${{ matrix.python-version }}
57+
dependency-version: ${{ matrix.dependency-version }}
58+
- name: Set up Python (cache hit — needed for the restored venv interpreter)
59+
if: steps.deps.outputs.cache-hit == 'true'
60+
uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5
61+
with:
62+
python-version: ${{ matrix.python-version }}
63+
- name: Run tests (offline, from prewarmed venv)
64+
if: steps.deps.outputs.cache-hit == 'true'
65+
run: .venv/bin/python -m pytest tests/unit -m "not realkernel"
66+
67+
# --- Fallback path: same-repo PRs / cache miss use JFrog (unchanged) --
2968
- name: Setup Poetry
69+
if: steps.deps.outputs.cache-hit != 'true'
3070
uses: ./.github/actions/setup-poetry
3171
with:
3272
python-version: ${{ matrix.python-version }}
3373
cache-suffix: "${{ matrix.dependency-version }}-"
3474
- name: Install Python tools for custom versions
35-
if: matrix.dependency-version != 'default'
75+
if: steps.deps.outputs.cache-hit != 'true' && matrix.dependency-version != 'default'
3676
run: poetry run pip install toml packaging
3777
- name: Generate requirements file
38-
if: matrix.dependency-version != 'default'
78+
if: steps.deps.outputs.cache-hit != 'true' && matrix.dependency-version != 'default'
3979
run: |
4080
poetry run python scripts/dependency_manager.py ${{ matrix.dependency-version }} --output requirements-${{ matrix.dependency-version }}.txt
4181
echo "Generated requirements for ${{ matrix.dependency-version }} versions:"
4282
cat requirements-${{ matrix.dependency-version }}.txt
4383
- name: Override with custom dependency versions
44-
if: matrix.dependency-version != 'default'
84+
if: steps.deps.outputs.cache-hit != 'true' && matrix.dependency-version != 'default'
4585
run: poetry run pip install -r requirements-${{ matrix.dependency-version }}.txt
4686
- name: Show installed versions
87+
if: steps.deps.outputs.cache-hit != 'true'
4788
run: |
4889
echo "=== Dependency Version: ${{ matrix.dependency-version }} ==="
4990
poetry run pip list
5091
- name: Run tests
51-
run: poetry run python -m pytest tests/unit
92+
if: steps.deps.outputs.cache-hit != 'true'
93+
run: poetry run python -m pytest tests/unit -m "not realkernel"
5294

5395
run-unit-tests-with-arrow:
5496
runs-on:
@@ -69,34 +111,146 @@ jobs:
69111
steps:
70112
- name: Check out repository
71113
uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
114+
# libkrb5 is a runtime system lib (not in the venv), so install it on
115+
# both the fork and fallback paths.
72116
- name: Install Kerberos system dependencies
73117
run: |
74118
sudo apt-get update
75119
sudo apt-get install -y libkrb5-dev
120+
121+
# --- Fork path: restore prewarmed pyarrow venv, run offline ------------
122+
- name: Restore prewarmed dependencies
123+
id: deps
124+
uses: ./.github/actions/restore-deps
125+
with:
126+
python-version: ${{ matrix.python-version }}
127+
dependency-version: ${{ matrix.dependency-version }}
128+
extras: pyarrow
129+
- name: Set up Python (cache hit — needed for the restored venv interpreter)
130+
if: steps.deps.outputs.cache-hit == 'true'
131+
uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5
132+
with:
133+
python-version: ${{ matrix.python-version }}
134+
- name: Run tests (offline, from prewarmed venv)
135+
if: steps.deps.outputs.cache-hit == 'true'
136+
run: .venv/bin/python -m pytest tests/unit -m "not realkernel"
137+
138+
# --- Fallback path: same-repo PRs / cache miss use JFrog (unchanged) ---
76139
- name: Setup Poetry
140+
if: steps.deps.outputs.cache-hit != 'true'
77141
uses: ./.github/actions/setup-poetry
78142
with:
79143
python-version: ${{ matrix.python-version }}
80-
install-args: "--all-extras"
144+
# Install ONLY the pyarrow extra (not --all-extras) so this
145+
# tier isolates the "pyarrow present, kernel absent"
146+
# configuration. --all-extras would also pull the kernel wheel,
147+
# making this job redundant with "Unit Tests + Kernel".
148+
install-args: "--extras pyarrow"
81149
cache-suffix: "pyarrow-${{ matrix.dependency-version }}-"
82150
- name: Install Python tools for custom versions
83-
if: matrix.dependency-version != 'default'
151+
if: steps.deps.outputs.cache-hit != 'true' && matrix.dependency-version != 'default'
84152
run: poetry run pip install toml packaging
85153
- name: Generate requirements file with pyarrow
86-
if: matrix.dependency-version != 'default'
154+
if: steps.deps.outputs.cache-hit != 'true' && matrix.dependency-version != 'default'
87155
run: |
88156
poetry run python scripts/dependency_manager.py ${{ matrix.dependency-version }} --output requirements-${{ matrix.dependency-version }}-arrow.txt
89157
echo "Generated requirements for ${{ matrix.dependency-version }} versions with PyArrow:"
90158
cat requirements-${{ matrix.dependency-version }}-arrow.txt
91159
- name: Override with custom dependency versions
92-
if: matrix.dependency-version != 'default'
160+
if: steps.deps.outputs.cache-hit != 'true' && matrix.dependency-version != 'default'
93161
run: poetry run pip install -r requirements-${{ matrix.dependency-version }}-arrow.txt
94162
- name: Show installed versions
163+
if: steps.deps.outputs.cache-hit != 'true'
95164
run: |
96165
echo "=== Dependency Version: ${{ matrix.dependency-version }} with PyArrow ==="
97166
poetry run pip list
98167
- name: Run tests
99-
run: poetry run python -m pytest tests/unit
168+
if: steps.deps.outputs.cache-hit != 'true'
169+
run: poetry run python -m pytest tests/unit -m "not realkernel"
170+
171+
run-unit-tests-with-kernel:
172+
runs-on:
173+
group: databricks-protected-runner-group
174+
labels: linux-ubuntu-latest
175+
strategy:
176+
matrix:
177+
# Kernel wheel is cp310-abi3 (Requires-Python >=3.10), so this
178+
# matrix omits 3.9 — the [kernel] extra is a no-op there.
179+
python-version: ["3.10", "3.11", "3.12", "3.13", "3.14"]
180+
181+
name: "Unit Tests + Kernel (Python ${{ matrix.python-version }})"
182+
# base restore-deps entries omit 3.9-kernel (no kernel wheel there); this
183+
# matrix already starts at 3.10, so every leg has a warmed counterpart.
184+
185+
steps:
186+
- name: Check out repository
187+
uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
188+
- name: Install Kerberos system dependencies
189+
run: |
190+
sudo apt-get update
191+
sudo apt-get install -y libkrb5-dev
192+
193+
# --- Fork path: restore prewarmed kernel venv, run offline ------------
194+
# NOTE: the [kernel] extra pulls databricks-sql-kernel, the one PRIVATE
195+
# (JFrog-only) package. Baking it into the warmed venv is precisely what
196+
# lets a fork exercise the kernel routing without any JFrog credential.
197+
- name: Restore prewarmed dependencies
198+
id: deps
199+
uses: ./.github/actions/restore-deps
200+
with:
201+
python-version: ${{ matrix.python-version }}
202+
extras: kernel
203+
- name: Set up Python (cache hit — needed for the restored venv interpreter)
204+
if: steps.deps.outputs.cache-hit == 'true'
205+
uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5
206+
with:
207+
python-version: ${{ matrix.python-version }}
208+
- name: Assert the real kernel wheel is installed (offline)
209+
if: steps.deps.outputs.cache-hit == 'true'
210+
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__)"
211+
- name: Unit tests, realkernel deselected (offline)
212+
if: steps.deps.outputs.cache-hit == 'true'
213+
run: .venv/bin/python -m pytest tests/unit -m "not realkernel"
214+
- name: Drive use_kernel=True through the REAL wheel — routing (offline)
215+
if: steps.deps.outputs.cache-hit == 'true'
216+
run: .venv/bin/python -m pytest tests/unit/test_session.py -m realkernel -v
217+
218+
# --- Fallback path: same-repo PRs / cache miss use JFrog (unchanged) --
219+
- name: Setup Poetry
220+
if: steps.deps.outputs.cache-hit != 'true'
221+
uses: ./.github/actions/setup-poetry
222+
with:
223+
python-version: ${{ matrix.python-version }}
224+
# Install the kernel extra (pulls the published
225+
# databricks-sql-kernel wheel, which transitively brings
226+
# pyarrow). Explicit --extras kernel rather than --all-extras
227+
# so this tier targets the kernel configuration specifically.
228+
install-args: "--extras kernel"
229+
cache-suffix: "kernel-"
230+
- name: Show installed versions
231+
if: steps.deps.outputs.cache-hit != 'true'
232+
run: |
233+
echo "=== with databricks-sql-kernel ==="
234+
poetry run pip list
235+
- name: Assert the real kernel wheel is installed (not a stub)
236+
if: steps.deps.outputs.cache-hit != 'true'
237+
run: |
238+
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__)"
239+
- name: Unit tests (kernel wheel present, realkernel deselected)
240+
if: steps.deps.outputs.cache-hit != 'true'
241+
# The bulk of tests/unit fakes databricks_sql_kernel in
242+
# sys.modules, so the real-wheel routing test is deselected here
243+
# and run on its own below (a shared session would shadow the
244+
# real wheel — both real-wheel tests fail loudly if that happens).
245+
run: poetry run python -m pytest tests/unit -m "not realkernel"
246+
- name: Drive use_kernel=True through the REAL wheel (routing)
247+
if: steps.deps.outputs.cache-hit != 'true'
248+
# Separate invocation, explicit file path: never collects the
249+
# fake-module test file, so sys.modules stays unpolluted. This is
250+
# the no-network proof that sql.connect(use_kernel=True) actually
251+
# instantiates the real KernelDatabricksClient (not a stub, not a
252+
# Thrift fallback). Fails loudly if the real wheel is shadowed.
253+
run: poetry run python -m pytest tests/unit/test_session.py -m realkernel -v
100254

101255
check-linting:
102256
runs-on:
@@ -108,11 +262,26 @@ jobs:
108262
steps:
109263
- name: Check out repository
110264
uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
265+
- name: Restore prewarmed dependencies
266+
id: deps
267+
uses: ./.github/actions/restore-deps
268+
with:
269+
python-version: ${{ matrix.python-version }}
270+
- name: Set up Python (cache hit — needed for the restored venv interpreter)
271+
if: steps.deps.outputs.cache-hit == 'true'
272+
uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5
273+
with:
274+
python-version: ${{ matrix.python-version }}
275+
- name: Black (offline, from prewarmed venv)
276+
if: steps.deps.outputs.cache-hit == 'true'
277+
run: .venv/bin/black --check src
111278
- name: Setup Poetry
279+
if: steps.deps.outputs.cache-hit != 'true'
112280
uses: ./.github/actions/setup-poetry
113281
with:
114282
python-version: ${{ matrix.python-version }}
115283
- name: Black
284+
if: steps.deps.outputs.cache-hit != 'true'
116285
run: poetry run black --check src
117286

118287
check-types:
@@ -125,11 +294,30 @@ jobs:
125294
steps:
126295
- name: Check out repository
127296
uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
297+
- name: Restore prewarmed dependencies
298+
id: deps
299+
uses: ./.github/actions/restore-deps
300+
with:
301+
python-version: ${{ matrix.python-version }}
302+
- name: Set up Python (cache hit — needed for the restored venv interpreter)
303+
if: steps.deps.outputs.cache-hit == 'true'
304+
uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5
305+
with:
306+
python-version: ${{ matrix.python-version }}
307+
- name: Mypy (offline, from prewarmed venv)
308+
if: steps.deps.outputs.cache-hit == 'true'
309+
# --install-types would reach the index; the warmed venv already has
310+
# the stub packages, so run mypy without it on the offline path.
311+
run: |
312+
mkdir -p .mypy_cache
313+
.venv/bin/mypy --non-interactive src
128314
- name: Setup Poetry
315+
if: steps.deps.outputs.cache-hit != 'true'
129316
uses: ./.github/actions/setup-poetry
130317
with:
131318
python-version: ${{ matrix.python-version }}
132319
- name: Mypy
320+
if: steps.deps.outputs.cache-hit != 'true'
133321
run: |
134322
mkdir .mypy_cache
135323
poetry run mypy --install-types --non-interactive src

0 commit comments

Comments
 (0)