Skip to content

Commit 665b370

Browse files
also de-collide UC Volume tests and add PR-branch concurrency cancellation
While verifying the transactions fix, a second flake surfaced — the same cross-CI race pattern, this time on a UC Volume file path: tests/e2e/test_driver.py::TestPySQLCoreSuite:: test_uc_volume_put_fails_if_file_exists_and_overwrite_not_set → Failed: DID NOT RAISE <class 'databricks.sql.exc.ServerOperationError'> Two tests in `tests/e2e/common/uc_volume_tests.py` PUT/GET/REMOVE against the hardcoded path `/Volumes/{catalog}/{schema}/e2etests/file1.csv`. When two CI runs overlap (as happened on this PR after the force-push for DCO sign-off), one run's REMOVE deletes the other run's file between its two PUTs, so the expected `FILE_IN_STAGING_PATH_ALREADY_EXISTS` never fires. Suffix the volume path with `uuid4().hex[:8]` in the two affected tests (test_uc_volume_life_cycle and test_uc_volume_put_fails_if_file_exists_ and_overwrite_not_set). The other UC Volume tests that reference the same path are exercising client-side / server-parse failure paths that never touch the file — leaving those alone keeps the diff focused. Also add a `concurrency` block to the E2E workflow: - On pull_request, cancel-in-progress: a new push / force-push on a PR cancels the previous run on that ref. Eliminates the entire class of "force-pushed during CI → two runs racing on shared warehouse state" failures. - On push to main, runs are NOT cancelled — each merge commit keeps its own clean CI signal so a regression on commit N can't be hidden by commit N+1 landing seconds later. Concurrent main runs can still collide on shared state, but the uuid-suffix conventions in the e2e tests are what keep that isolated. Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
1 parent ce5b170 commit 665b370

2 files changed

Lines changed: 29 additions & 8 deletions

File tree

.github/workflows/code-coverage.yml

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,19 @@ on:
1111
pull_request:
1212
workflow_dispatch:
1313

14+
# Serialise E2E runs per ref so a force-push (or a fast follow-up commit)
15+
# on a PR cancels the previous run instead of racing it against shared
16+
# warehouse state (Delta tables, UC Volume files, etc.).
17+
#
18+
# Pushes to main are NOT cancelled — each merge commit needs its own clean
19+
# CI signal so a regression on commit N doesn't get hidden by commit N+1
20+
# arriving seconds later. (Concurrent main runs can still collide on shared
21+
# state, but that's the cost of preserving per-commit signal; the
22+
# uuid-suffix conventions in the e2e tests are what keep them isolated.)
23+
concurrency:
24+
group: e2e-${{ github.workflow }}-${{ github.ref }}
25+
cancel-in-progress: ${{ github.event_name == 'pull_request' }}
26+
1427
jobs:
1528
test-with-coverage:
1629
runs-on:

tests/e2e/common/uc_volume_tests.py

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import os
22
import tempfile
3+
from uuid import uuid4
34

45
import pytest
56
import databricks.sql as sql
@@ -40,12 +41,16 @@ def test_uc_volume_life_cycle(self, catalog, schema):
4041
with open(fh, "wb") as fp:
4142
fp.write(original_text)
4243

44+
# Unique per-run path so concurrent CI jobs sharing the same volume
45+
# don't step on each other's PUT/GET/REMOVE.
46+
volume_path = f"/Volumes/{catalog}/{schema}/e2etests/life_cycle_{uuid4().hex[:8]}.csv"
47+
4348
with self.connection(
4449
extra_params={"staging_allowed_local_path": temp_path}
4550
) as conn:
4651

4752
cursor = conn.cursor()
48-
query = f"PUT '{temp_path}' INTO '/Volumes/{catalog}/{schema}/e2etests/file1.csv' OVERWRITE"
53+
query = f"PUT '{temp_path}' INTO '{volume_path}' OVERWRITE"
4954
cursor.execute(query)
5055

5156
# GET should succeed
@@ -56,7 +61,7 @@ def test_uc_volume_life_cycle(self, catalog, schema):
5661
extra_params={"staging_allowed_local_path": new_temp_path}
5762
) as conn:
5863
cursor = conn.cursor()
59-
query = f"GET '/Volumes/{catalog}/{schema}/e2etests/file1.csv' TO '{new_temp_path}'"
64+
query = f"GET '{volume_path}' TO '{new_temp_path}'"
6065
cursor.execute(query)
6166

6267
with open(new_fh, "rb") as fp:
@@ -66,7 +71,7 @@ def test_uc_volume_life_cycle(self, catalog, schema):
6671

6772
# REMOVE should succeed
6873

69-
remove_query = f"REMOVE '/Volumes/{catalog}/{schema}/e2etests/file1.csv'"
74+
remove_query = f"REMOVE '{volume_path}'"
7075

7176
# Use minimal retry settings to fail fast
7277
extra_params = {
@@ -84,7 +89,7 @@ def test_uc_volume_life_cycle(self, catalog, schema):
8489
Error, match="Staging operation over HTTP was unsuccessful: 404"
8590
):
8691
cursor = conn.cursor()
87-
query = f"GET '/Volumes/{catalog}/{schema}/e2etests/file1.csv' TO '{new_temp_path}'"
92+
query = f"GET '{volume_path}' TO '{new_temp_path}'"
8893
cursor.execute(query)
8994

9095
os.remove(temp_path)
@@ -151,19 +156,22 @@ def test_uc_volume_put_fails_if_file_exists_and_overwrite_not_set(
151156
with open(fh, "wb") as fp:
152157
fp.write(original_text)
153158

159+
# Unique per-run path so a concurrent CI job's REMOVE doesn't delete
160+
# our file between the two PUTs and silently turn the expected
161+
# FILE_IN_STAGING_PATH_ALREADY_EXISTS into a successful PUT.
162+
volume_path = f"/Volumes/{catalog}/{schema}/e2etests/put_conflict_{uuid4().hex[:8]}.csv"
163+
154164
def perform_put():
155165
with self.connection(
156166
extra_params={"staging_allowed_local_path": temp_path}
157167
) as conn:
158168
cursor = conn.cursor()
159-
query = f"PUT '{temp_path}' INTO '/Volumes/{catalog}/{schema}/e2etests/file1.csv'"
169+
query = f"PUT '{temp_path}' INTO '{volume_path}'"
160170
cursor.execute(query)
161171

162172
def perform_remove():
163173
try:
164-
remove_query = (
165-
f"REMOVE '/Volumes/{catalog}/{schema}/e2etests/file1.csv'"
166-
)
174+
remove_query = f"REMOVE '{volume_path}'"
167175

168176
with self.connection(
169177
extra_params={"staging_allowed_local_path": "/"}

0 commit comments

Comments
 (0)