Skip to content

Commit 17b5b00

Browse files
Optimize CI: consolidate workflows, fix caching, speed up e2e tests
Workflow consolidation: - Delete integration.yml and daily-telemetry-e2e.yml (redundant with coverage workflow which already runs all e2e tests) - Add push-to-main trigger to coverage workflow - Run all tests (including telemetry) in single pytest invocation with --dist=loadgroup to respect xdist_group markers for isolation Fix pyarrow cache: - Remove cache-path: .venv-pyarrow from pyarrow jobs. Poetry always creates .venv regardless of the cache-path input, so the cache was never saved ("Path does not exist" error). The cache-suffix already differentiates keys between variants. Fix 3.14 post-test DNS hang: - Add enable_telemetry=False to unit test DUMMY_CONNECTION_ARGS that use server_hostname="foo". This prevents FeatureFlagsContext from making real HTTP calls to fake hosts, eliminating ~8min hang from ThreadPoolExecutor threads timing out on DNS on protected runners. Improve e2e test parallelization: - Split TestPySQLLargeQueriesSuite into 3 separate classes (TestPySQLLargeWideResultSet, TestPySQLLargeNarrowResultSet, TestPySQLLongRunningQuery) so xdist distributes them across workers instead of all landing on one. Speed up slow tests: - Reduce large result set sizes from 300MB to 100MB (still validates large fetches, lz4, chunking, row integrity) - Start test_long_running_query at scale_factor=50 instead of 1 to skip ramp-up iterations that finish instantly Co-authored-by: Isaac Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
1 parent 23489a6 commit 17b5b00

File tree

8 files changed

+58
-156
lines changed

8 files changed

+58
-156
lines changed

.github/workflows/code-coverage.yml

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,15 @@
1-
name: Code Coverage
1+
name: E2E Tests and Code Coverage
22

33
permissions:
44
contents: read
55
id-token: write
66

7-
on: [pull_request, workflow_dispatch]
7+
on:
8+
push:
9+
branches:
10+
- main
11+
pull_request:
12+
workflow_dispatch:
813

914
jobs:
1015
test-with-coverage:
@@ -32,25 +37,16 @@ jobs:
3237
with:
3338
python-version: "3.10"
3439
install-args: "--all-extras"
35-
- name: Run parallel tests with coverage
40+
- name: Run all tests with coverage
3641
continue-on-error: false
3742
run: |
3843
poetry run pytest tests/unit tests/e2e \
39-
-m "not serial" \
4044
-n auto \
45+
--dist=loadgroup \
4146
--cov=src \
4247
--cov-report=xml \
4348
--cov-report=term \
4449
-v
45-
- name: Run telemetry tests with coverage (isolated)
46-
continue-on-error: false
47-
run: |
48-
poetry run pytest tests/e2e/test_concurrent_telemetry.py \
49-
--cov=src \
50-
--cov-append \
51-
--cov-report=xml \
52-
--cov-report=term \
53-
-v
5450
- name: Check for coverage override
5551
id: override
5652
env:

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,6 @@ jobs:
7878
with:
7979
python-version: ${{ matrix.python-version }}
8080
install-args: "--all-extras"
81-
cache-path: ".venv-pyarrow"
8281
cache-suffix: "pyarrow-${{ matrix.dependency-version }}-"
8382
- name: Install Python tools for custom versions
8483
if: matrix.dependency-version != 'default'

.github/workflows/daily-telemetry-e2e.yml

Lines changed: 0 additions & 58 deletions
This file was deleted.

.github/workflows/integration.yml

Lines changed: 0 additions & 71 deletions
This file was deleted.

tests/e2e/common/large_queries_mixin.py

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,8 @@
77
log = logging.getLogger(__name__)
88

99

10-
class LargeQueriesMixin:
11-
"""
12-
This mixin expects to be mixed with a CursorTest-like class
13-
"""
10+
class LargeQueriesFetchMixin:
11+
"""Shared fetch helper for large query test classes."""
1412

1513
def fetch_rows(self, cursor, row_count, fetchmany_size):
1614
"""
@@ -44,6 +42,10 @@ def fetch_rows(self, cursor, row_count, fetchmany_size):
4442
+ "assuming 10K fetch size."
4543
)
4644

45+
46+
class LargeWideResultSetMixin(LargeQueriesFetchMixin):
47+
"""Test mixin for large wide result set queries."""
48+
4749
@pytest.mark.parametrize(
4850
"extra_params",
4951
[
@@ -52,7 +54,7 @@ def fetch_rows(self, cursor, row_count, fetchmany_size):
5254
],
5355
)
5456
def test_query_with_large_wide_result_set(self, extra_params):
55-
resultSize = 300 * 1000 * 1000 # 300 MB
57+
resultSize = 100 * 1000 * 1000 # 100 MB
5658
width = 8192 # B
5759
rows = resultSize // width
5860
cols = width // 36
@@ -77,6 +79,10 @@ def test_query_with_large_wide_result_set(self, extra_params):
7779
assert row[0] == row_id # Verify no rows are dropped in the middle.
7880
assert len(row[1]) == 36
7981

82+
83+
class LargeNarrowResultSetMixin(LargeQueriesFetchMixin):
84+
"""Test mixin for large narrow result set queries."""
85+
8086
@pytest.mark.parametrize(
8187
"extra_params",
8288
[
@@ -85,7 +91,7 @@ def test_query_with_large_wide_result_set(self, extra_params):
8591
],
8692
)
8793
def test_query_with_large_narrow_result_set(self, extra_params):
88-
resultSize = 300 * 1000 * 1000 # 300 MB
94+
resultSize = 100 * 1000 * 1000 # 100 MB
8995
width = 8 # sizeof(long)
9096
rows = resultSize / width
9197

@@ -98,6 +104,10 @@ def test_query_with_large_narrow_result_set(self, extra_params):
98104
for row_id, row in enumerate(self.fetch_rows(cursor, rows, fetchmany_size)):
99105
assert row[0] == row_id
100106

107+
108+
class LongRunningQueryMixin:
109+
"""Test mixin for long running queries."""
110+
101111
@pytest.mark.parametrize(
102112
"extra_params",
103113
[
@@ -114,7 +124,7 @@ def test_long_running_query(self, extra_params):
114124

115125
duration = -1
116126
scale0 = 10000
117-
scale_factor = 1
127+
scale_factor = 50
118128
with self.cursor(extra_params) as cursor:
119129
while duration < min_duration:
120130
assert scale_factor < 4096, "Detected infinite loop"
@@ -138,3 +148,8 @@ def test_long_running_query(self, extra_params):
138148
print("Took {} s with scale factor={}".format(duration, scale_factor))
139149
# Extrapolate linearly to reach 3 min and add 50% padding to push over the limit
140150
scale_factor = math.ceil(1.5 * scale_factor / current_fraction)
151+
152+
153+
# Keep backward-compatible alias that combines all three
154+
class LargeQueriesMixin(LargeWideResultSetMixin, LargeNarrowResultSetMixin, LongRunningQueryMixin):
155+
pass

tests/e2e/test_driver.py

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,11 @@
3939
)
4040
from databricks.sql.thrift_api.TCLIService import ttypes
4141
from tests.e2e.common.core_tests import CoreTestMixin, SmokeTestMixin
42-
from tests.e2e.common.large_queries_mixin import LargeQueriesMixin
42+
from tests.e2e.common.large_queries_mixin import (
43+
LargeWideResultSetMixin,
44+
LargeNarrowResultSetMixin,
45+
LongRunningQueryMixin,
46+
)
4347
from tests.e2e.common.timestamp_tests import TimestampTestsMixin
4448
from tests.e2e.common.decimal_tests import DecimalTestsMixin
4549
from tests.e2e.common.retry_test_mixins import (
@@ -138,24 +142,36 @@ def assertEqualRowValues(self, actual, expected):
138142
assert act[i] == exp[i]
139143

140144

141-
class TestPySQLLargeQueriesSuite(PySQLPytestTestCase, LargeQueriesMixin):
145+
class _LargeQueryRowHelper:
146+
"""Shared helper for fetching rows one at a time in large query tests."""
147+
142148
def get_some_rows(self, cursor, fetchmany_size):
143149
row = cursor.fetchone()
144150
if row:
145151
return [row]
146152
else:
147153
return None
148154

155+
156+
class TestPySQLLargeWideResultSet(PySQLPytestTestCase, _LargeQueryRowHelper, LargeWideResultSetMixin):
157+
pass
158+
159+
160+
class TestPySQLLargeNarrowResultSet(PySQLPytestTestCase, _LargeQueryRowHelper, LargeNarrowResultSetMixin):
161+
pass
162+
163+
164+
class TestPySQLLongRunningQuery(PySQLPytestTestCase, LongRunningQueryMixin):
165+
pass
166+
167+
168+
class TestPySQLCloudFetch(PySQLPytestTestCase):
149169
@skipUnless(pysql_supports_arrow(), "needs arrow support")
150170
@pytest.mark.skip("This test requires a previously uploaded data set")
151171
def test_cloud_fetch(self):
152-
# This test can take several minutes to run
153172
limits = [100000, 300000]
154173
threads = [10, 25]
155174
self.arraysize = 100000
156-
# This test requires a large table with many rows to properly initiate cloud fetch.
157-
# e2-dogfood host > hive_metastore catalog > main schema has such a table called store_sales.
158-
# If this table is deleted or this test is run on a different host, a different table may need to be used.
159175
base_query = "SELECT * FROM store_sales WHERE ss_sold_date_sk = 2452234 "
160176
for num_limit, num_threads, lz4_compression in itertools.product(
161177
limits, threads, [True, False]

tests/unit/test_client.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,7 @@ class ClientTestSuite(unittest.TestCase):
8787
"server_hostname": "foo",
8888
"http_path": "dummy_path",
8989
"access_token": "tok",
90+
"enable_telemetry": False,
9091
}
9192

9293
@patch("%s.session.ThriftDatabricksClient" % PACKAGE_NAME)
@@ -644,6 +645,7 @@ class TransactionTestSuite(unittest.TestCase):
644645
"server_hostname": "foo",
645646
"http_path": "dummy_path",
646647
"access_token": "tok",
648+
"enable_telemetry": False,
647649
}
648650

649651
def _setup_mock_session_with_http_client(self, mock_session):

tests/unit/test_session.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ class TestSession:
2222
"server_hostname": "foo",
2323
"http_path": "dummy_path",
2424
"access_token": "tok",
25+
"enable_telemetry": False,
2526
}
2627

2728
@patch("%s.session.ThriftDatabricksClient" % PACKAGE_NAME)
@@ -50,13 +51,15 @@ def test_auth_args(self, mock_client_class):
5051
"server_hostname": "foo",
5152
"http_path": None,
5253
"access_token": "tok",
54+
"enable_telemetry": False,
5355
},
5456
{
5557
"server_hostname": "foo",
5658
"http_path": None,
5759
"_tls_client_cert_file": "something",
5860
"_use_cert_as_auth": True,
5961
"access_token": None,
62+
"enable_telemetry": False,
6063
},
6164
]
6265

0 commit comments

Comments
 (0)