From 68c2119554a27462626d06b3ec641e6f9a3cfa79 Mon Sep 17 00:00:00 2001 From: weishan <11898366+weiishann@users.noreply.github.com> Date: Sun, 20 Sep 2026 22:54:17 +0800 Subject: [PATCH 1/4] ci(build): retry the sbt launcher download The launcher jar is gitignored, so every CI job re-fetches it from Maven Central before sbt can start, and the curl call had no retry. A single transient error there failed the whole job: on PR #15 one matrix leg died in 70ms on this download and fail-fast then cancelled the other, so the pull request got no test signal at all. --retry covers 5xx and timeouts, --retry-all-errors extends that to connection resets, and --show-error stops --silent from swallowing the HTTP status when every attempt fails. dev/test-sbt-launch-lib.sh exercises both failure modes against a local server and was confirmed to fail before this change and pass after. Co-Authored-By: Claude Opus 5 (1M context) --- build/sbt-launch-lib.bash | 8 +- dev/test-sbt-launch-lib.sh | 191 +++++++++++++++++++++++++++++++++++++ 2 files changed, 198 insertions(+), 1 deletion(-) create mode 100755 dev/test-sbt-launch-lib.sh diff --git a/build/sbt-launch-lib.bash b/build/sbt-launch-lib.bash index 603b6316..662df60d 100755 --- a/build/sbt-launch-lib.bash +++ b/build/sbt-launch-lib.bash @@ -58,7 +58,13 @@ acquire_sbt_jar () { printf 'Attempting to fetch sbt from %s\n' "${URL1}" JAR_DL="${JAR}.part" if [ $(command -v curl) ]; then - curl --fail --location --silent ${URL1} > "${JAR_DL}" &&\ + # Retry: a single transient error from the artifact repository must not + # fail the build. --retry alone covers 5xx and timeouts; --retry-all-errors + # extends that to connection resets. --show-error keeps the HTTP status + # visible when every attempt fails. + curl --fail --location --silent --show-error \ + --retry 5 --retry-all-errors --connect-timeout 10 \ + ${URL1} > "${JAR_DL}" &&\ mv "${JAR_DL}" "${JAR}" elif [ $(command -v wget) ]; then wget --quiet ${URL1} -O "${JAR_DL}" &&\ diff --git a/dev/test-sbt-launch-lib.sh b/dev/test-sbt-launch-lib.sh new file mode 100755 index 00000000..b49a07a0 --- /dev/null +++ b/dev/test-sbt-launch-lib.sh @@ -0,0 +1,191 @@ +#!/usr/bin/env bash +# +# Tests for build/sbt-launch-lib.bash. +# +# Run directly: dev/test-sbt-launch-lib.sh +# +# The launcher jar is gitignored, so every CI job fetches it from Maven Central +# before sbt can start. A single transient failure there fails the whole job, so +# the fetch must survive one. + +set -uo pipefail + +REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +LIB="${REPO_ROOT}/build/sbt-launch-lib.bash" +SBT_VERSION="$(awk -F "=" '/sbt\.version/ {print $2}' "${REPO_ROOT}/project/build.properties")" + +failures=0 +server_pid="" +workspaces=() + +cleanup() { + [[ -n "${server_pid}" ]] && kill "${server_pid}" 2>/dev/null + for dir in "${workspaces[@]+"${workspaces[@]}"}"; do + rm -rf "${dir}" + done +} +trap cleanup EXIT + +fail() { + echo " FAIL: $*" + failures=$((failures + 1)) +} + +pass() { + echo " PASS: $*" +} + +wait_for_port() { + local port_file="$1" what="$2" + for _ in $(seq 1 50); do + [[ -s "${port_file}" ]] && return 0 + sleep 0.1 + done + echo "${what} failed to start" >&2 + return 1 +} + +# Serves $1 failing responses (HTTP 503) before serving the jar successfully. +start_flaky_server() { + local fail_count="$1" jar="$2" port_file="$3" + + python3 - "${fail_count}" "${jar}" "${port_file}" <<'FLAKY' & +import http.server, socketserver, sys + +fail_count, jar_path, port_file = int(sys.argv[1]), sys.argv[2], sys.argv[3] +hits = {"n": 0} + +class Handler(http.server.BaseHTTPRequestHandler): + protocol_version = "HTTP/1.0" + + def do_GET(self): + hits["n"] += 1 + if hits["n"] <= fail_count: + self.send_response(503) + self.send_header("Content-Length", "0") + self.end_headers() + return + with open(jar_path, "rb") as f: + body = f.read() + self.send_response(200) + self.send_header("Content-Length", str(len(body))) + self.end_headers() + self.wfile.write(body) + + def log_message(self, *args): + pass + +socketserver.TCPServer.allow_reuse_address = True +with socketserver.TCPServer(("127.0.0.1", 0), Handler) as httpd: + with open(port_file, "w") as f: + f.write(str(httpd.server_address[1])) + httpd.serve_forever() +FLAKY + server_pid=$! + # Keep the shell from reporting the job when cleanup kills it. + disown "${server_pid}" 2>/dev/null || true + wait_for_port "${port_file}" "flaky server" +} + +# Resets $1 connections before serving the jar successfully. A reset is not an +# HTTP status, so plain --retry does not cover it. +start_resetting_server() { + local fail_count="$1" jar="$2" port_file="$3" + + python3 - "${fail_count}" "${jar}" "${port_file}" <<'RESET' & +import socket, struct, sys + +fail_count, jar_path, port_file = int(sys.argv[1]), sys.argv[2], sys.argv[3] + +sock = socket.socket() +sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) +sock.bind(("127.0.0.1", 0)) +sock.listen(8) + +with open(port_file, "w") as f: + f.write(str(sock.getsockname()[1])) + +seen = 0 +while True: + conn, _ = sock.accept() + seen += 1 + if seen <= fail_count: + # SO_LINGER with a zero timeout makes close() send RST, not FIN. + conn.setsockopt(socket.SOL_SOCKET, socket.SO_LINGER, struct.pack("ii", 1, 0)) + conn.close() + continue + conn.recv(65535) + with open(jar_path, "rb") as f: + body = f.read() + conn.sendall(b"HTTP/1.0 200 OK\r\nContent-Length: %d\r\n\r\n" % len(body) + body) + conn.close() +RESET + server_pid=$! + disown "${server_pid}" 2>/dev/null || true + wait_for_port "${port_file}" "resetting server" +} + +# acquire_sbt_jar needs project/build.properties and a build/ directory relative +# to the working directory. +make_workspace() { + local dir + dir="$(mktemp -d)" + workspaces+=("${dir}") + mkdir -p "${dir}/project" "${dir}/build" + cp "${LIB}" "${dir}/build/sbt-launch-lib.bash" + echo "sbt.version=${SBT_VERSION}" > "${dir}/project/build.properties" + # Stand-in for the real launcher: acquire_sbt_jar only fetches bytes, it does + # not inspect them. + printf 'launcher-jar-contents' > "${dir}/upstream.jar" + echo "${dir}" +} + +# acquire_sbt_jar resolves paths relative to the working directory and calls +# `exit` on failure, so run it in a subshell rooted in the fixture. +run_acquire() { + local dir="$1" port="$2" + ( + cd "${dir}" || exit 1 + export DEFAULT_ARTIFACT_REPOSITORY="http://127.0.0.1:${port}" + # shellcheck disable=SC1091 + source build/sbt-launch-lib.bash + acquire_sbt_jar + ) > "${dir}/out.log" 2>&1 +} + +check_downloaded() { + local dir="$1" exit_code="$2" desc="$3" + local jar="${dir}/build/sbt-launch-${SBT_VERSION}.jar" + + if [[ ${exit_code} -ne 0 ]]; then + fail "expected exit 0 after the server recovered, got ${exit_code}" + sed 's/^/ /' "${dir}/out.log" + elif [[ ! -f "${jar}" ]]; then + fail "expected ${jar##*/} to be downloaded" + elif [[ "$(cat "${jar}")" != "launcher-jar-contents" ]]; then + fail "downloaded jar has unexpected contents" + else + pass "${desc}" + fi +} + +echo "acquire_sbt_jar recovers from transient HTTP errors" +work="$(make_workspace)" +start_flaky_server 2 "${work}/upstream.jar" "${work}/port" || exit 1 +run_acquire "${work}" "$(cat "${work}/port")" +check_downloaded "${work}" $? "retried past 2 responses of HTTP 503" +kill "${server_pid}" 2>/dev/null; server_pid="" + +echo "acquire_sbt_jar recovers from a connection reset" +work="$(make_workspace)" +start_resetting_server 2 "${work}/upstream.jar" "${work}/port" || exit 1 +run_acquire "${work}" "$(cat "${work}/port")" +check_downloaded "${work}" $? "retried past 2 connection resets" +kill "${server_pid}" 2>/dev/null; server_pid="" + +echo +if [[ ${failures} -gt 0 ]]; then + echo "${failures} test(s) failed" + exit 1 +fi +echo "All tests passed" From e863b1cf77bc9a91994ffa57cdcb393504807963 Mon Sep 17 00:00:00 2001 From: weishan <11898366+weiishann@users.noreply.github.com> Date: Sun, 20 Sep 2026 22:54:17 +0800 Subject: [PATCH 2/4] ci: run the Spark-independent modules once instead of once per Spark version Measured against a passing run, each matrix leg took ~22.5 minutes: ~16 for +jacoco and ~4.4 for licenseCheck. Only `spark` and `integrationTests` read sparkVersion, yet `+jacoco` ran the whole root aggregate in both legs, so the ~7 minutes of server and cli tests were executed twice, identically. unit-tests.yml now has a `core` job that runs the eight Spark-independent modules once, and a `spark` job that matrixes only the modules that depend on the Spark version. Together they cover exactly root's .aggregate(...) - no module is dropped and none runs twice. The `spark` job keeps the job names `test (Spark 4.0.0)` and `test (Spark 4.1.0)` verbatim because both are required status checks on main; renaming them would leave every pull request waiting on checks that never report. `core` is deliberately not added to branch protection yet. licenseCheck moves to its own workflow, gated on the build files, since dependency licences can only change when those change. Also, across the workflows: - cache ~/.sbt, ~/.ivy2/cache and ~/.cache/coursier, plus the launcher jar - fail-fast: false, so one leg's flake no longer cancels the other's results - concurrency groups, matching the idiom already used elsewhere in the repo - timeout-minutes on every job, sized to ~3-4x the measured runtime - upload the jacoco reports, which were previously written and discarded - run the format and UI checks on branch-* too, as the other workflows do Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/license-check.yml | 52 +++++++++++++ .github/workflows/lint-check.yml | 20 ++++- .github/workflows/python-client-tests.yml | 13 ++++ .github/workflows/ui-tests.yml | 9 ++- .github/workflows/unit-tests.yml | 95 +++++++++++++++++++++-- 5 files changed, 178 insertions(+), 11 deletions(-) create mode 100644 .github/workflows/license-check.yml diff --git a/.github/workflows/license-check.yml b/.github/workflows/license-check.yml new file mode 100644 index 00000000..8effae64 --- /dev/null +++ b/.github/workflows/license-check.yml @@ -0,0 +1,52 @@ +# Dependency licences can only change when the build definition changes, so this +# runs on the build files rather than on every pull request. It used to be a step +# inside the unit test matrix, where it cost ~4.5 minutes per leg on every change. + +name: License Check + +on: + push: + branches: [ "main", "branch-*" ] + paths: + - build.sbt + - version.sbt + - project/** + - .github/workflows/license-check.yml + pull_request: + branches: [ "main", "branch-*" ] + paths: + - build.sbt + - version.sbt + - project/** + - .github/workflows/license-check.yml + +permissions: + contents: read + +concurrency: + group: ${{ github.workflow }}-${{ github.event_name }}-${{ github.ref }} + cancel-in-progress: true + +jobs: + license-check: + runs-on: ubuntu-latest + timeout-minutes: 30 + + steps: + - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4.3.1 + - name: Set up JDK 17 + uses: actions/setup-java@c1e323688fd81a25caa38c78aa6df2d33d3e20d9 # v4.8.0 + with: + java-version: '17' + distribution: 'temurin' + # Caches ~/.sbt, ~/.ivy2/cache and ~/.cache/coursier. + cache: 'sbt' + + - name: Cache the sbt launcher + uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 + with: + path: build/sbt-launch-*.jar + key: sbt-launcher-${{ hashFiles('project/build.properties') }} + + - name: Run license check + run: build/sbt licenseCheck diff --git a/.github/workflows/lint-check.yml b/.github/workflows/lint-check.yml index 84092000..937b183e 100644 --- a/.github/workflows/lint-check.yml +++ b/.github/workflows/lint-check.yml @@ -2,13 +2,18 @@ name: Code Format Check on: push: - branches: [ "main" ] + branches: [ "main", "branch-*" ] pull_request: - branches: [ "main" ] + branches: [ "main", "branch-*" ] + +concurrency: + group: ${{ github.workflow }}-${{ github.event_name }}-${{ github.ref }} + cancel-in-progress: true jobs: lint: runs-on: ubuntu-latest + timeout-minutes: 15 permissions: contents: read pull-requests: read @@ -20,6 +25,8 @@ jobs: uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4.4.0 with: node-version: '18' + cache: 'yarn' + cache-dependency-path: ui/yarn.lock - name: Run lint check run: | @@ -27,6 +34,7 @@ jobs: java-fmt: runs-on: ubuntu-latest + timeout-minutes: 15 permissions: contents: read pull-requests: read @@ -39,6 +47,14 @@ jobs: with: java-version: '17' distribution: 'temurin' + # Caches ~/.sbt, ~/.ivy2/cache and ~/.cache/coursier. + cache: 'sbt' + + - name: Cache the sbt launcher + uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 + with: + path: build/sbt-launch-*.jar + key: sbt-launcher-${{ hashFiles('project/build.properties') }} - name: Run Java format check run: | diff --git a/.github/workflows/python-client-tests.yml b/.github/workflows/python-client-tests.yml index 403c415d..d33e341c 100644 --- a/.github/workflows/python-client-tests.yml +++ b/.github/workflows/python-client-tests.yml @@ -9,9 +9,14 @@ on: permissions: contents: read +concurrency: + group: ${{ github.workflow }}-${{ github.event_name }}-${{ github.ref }} + cancel-in-progress: true + jobs: test: runs-on: ubuntu-latest + timeout-minutes: 30 steps: - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4.3.1 @@ -20,6 +25,14 @@ jobs: with: java-version: '17' distribution: 'temurin' + # Caches ~/.sbt, ~/.ivy2/cache and ~/.cache/coursier. + cache: 'sbt' + + - name: Cache the sbt launcher + uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 + with: + path: build/sbt-launch-*.jar + key: sbt-launcher-${{ hashFiles('project/build.properties') }} - name: Install uv run: bash dev/install-uv.sh diff --git a/.github/workflows/ui-tests.yml b/.github/workflows/ui-tests.yml index 84807e16..a65ee0fc 100644 --- a/.github/workflows/ui-tests.yml +++ b/.github/workflows/ui-tests.yml @@ -2,19 +2,24 @@ name: UI Tests on: push: - branches: [ "main" ] + branches: [ "main", "branch-*" ] paths: - 'ui/**' - '.github/workflows/ui-tests.yml' pull_request: - branches: [ "main" ] + branches: [ "main", "branch-*" ] paths: - 'ui/**' - '.github/workflows/ui-tests.yml' +concurrency: + group: ${{ github.workflow }}-${{ github.event_name }}-${{ github.ref }} + cancel-in-progress: true + jobs: ui-test: runs-on: ubuntu-latest + timeout-minutes: 20 permissions: contents: read pull-requests: read diff --git a/.github/workflows/unit-tests.yml b/.github/workflows/unit-tests.yml index 322875ca..a392aead 100644 --- a/.github/workflows/unit-tests.yml +++ b/.github/workflows/unit-tests.yml @@ -13,12 +13,80 @@ on: permissions: contents: read +concurrency: + group: ${{ github.workflow }}-${{ github.event_name }}-${{ github.ref }} + cancel-in-progress: true + jobs: - test: + # Everything that does not depend on sparkVersion, run once. + # + # Only `spark` and `integrationTests` read sparkVersion (build.sbt), so these + # modules produce identical results for every Spark version. Running them + # inside the matrix duplicated ~7 minutes of tests, plus a ~4 minute + # licenseCheck, on every pull request. + # + # This list must stay in sync with root's .aggregate(...) in build.sbt, minus + # `spark`. A module added there and not added here is silently never tested. + core: + runs-on: ubuntu-latest + timeout-minutes: 45 + name: core + + steps: + - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4.3.1 + - name: Set up JDK 17 + uses: actions/setup-java@c1e323688fd81a25caa38c78aa6df2d33d3e20d9 # v4.8.0 + with: + java-version: '17' + distribution: 'temurin' + # Caches ~/.sbt, ~/.ivy2/cache and ~/.cache/coursier. + cache: 'sbt' + + - name: Cache the sbt launcher + uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 + with: + path: build/sbt-launch-*.jar + key: sbt-launcher-${{ hashFiles('project/build.properties') }} + + - name: Build and verify the local deployment. + run: build/sbt clean package publishM2 + + - name: Run tests with coverage + run: > + build/sbt -J-Xmx2G + +serverModels/jacoco + +client/jacoco + +pythonClient/jacoco + +server/jacoco + +cli/jacoco + +controlApi/jacoco + +controlModels/jacoco + +apiDocs/jacoco + + # jacoco writes a report per module and nothing consumed them before, so a + # coverage run produced numbers nobody could read. + - name: Upload coverage reports + if: always() + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: jacoco-core + path: '**/jacoco/report/**' + if-no-files-found: warn + retention-days: 7 + + # Only the Spark-dependent modules are matrixed. + # + # The job name is load-bearing: `test (Spark 4.0.0)` and `test (Spark 4.1.0)` + # are required status checks on main. Renaming them blocks every pull request + # on checks that never report. + spark: runs-on: ubuntu-latest + timeout-minutes: 45 name: test (Spark ${{ matrix.spark-version }}) strategy: + # A flake in one leg must not cancel the other leg's results. + fail-fast: false matrix: spark-version: ["4.0.0", "4.1.0"] @@ -29,17 +97,30 @@ jobs: with: java-version: '17' distribution: 'temurin' + # Caches ~/.sbt, ~/.ivy2/cache and ~/.cache/coursier. + cache: 'sbt' + + - name: Cache the sbt launcher + uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 + with: + path: build/sbt-launch-*.jar + key: sbt-launcher-${{ hashFiles('project/build.properties') }} # 'integrationTests' is not in root project, since we avoid running it via `./build/sbt test`, # Here, we explicitly compile `integrationTests` to catch compilation issues early. - name: Compile integration tests to verify compilation and checkstyle issues. run: build/sbt -DsparkVersion=${{ matrix.spark-version }} integrationTests/test:compile - - name: Build and verify the local deployment. - run: build/sbt -DsparkVersion=${{ matrix.spark-version }} clean package publishM2 - - name: Run tests with coverage - run: build/sbt -DsparkVersion=${{ matrix.spark-version }} -J-Xmx2G +jacoco + run: build/sbt -DsparkVersion=${{ matrix.spark-version }} -J-Xmx2G +spark/jacoco - - name: Run license check - run: build/sbt -DsparkVersion=${{ matrix.spark-version }} licenseCheck + # jacoco writes a report per module and nothing consumed them before, so a + # coverage run produced numbers nobody could read. + - name: Upload coverage reports + if: always() + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: jacoco-spark-${{ matrix.spark-version }} + path: '**/jacoco/report/**' + if-no-files-found: warn + retention-days: 7 From 34b57883227214557d32050ee64659698b93eef8 Mon Sep 17 00:00:00 2001 From: weishan <11898366+weiishann@users.noreply.github.com> Date: Sun, 20 Sep 2026 22:54:17 +0800 Subject: [PATCH 3/4] test(spark): stop Spark's own logging from flooding the CI log In a passing unit-test job, 226,580 of 240,773 log lines were Spark INFO messages. Spark logs to stderr, which sbt tags [error], so a green run was full of [error] lines and a real failure was effectively unfindable. All of that volume came from the Spark connector tests - the server and cli phases contributed none. There was no log4j2 config on the test classpath, so Spark fell back to its bundled defaults. This keeps the framework at WARN and io.unitycatalog at INFO, so Unity Catalog's own diagnostics survive. Measured on SchemaOperationsTest: log lines drop 77% (878 -> 203) with the same result, 5 of 5 passing. Co-Authored-By: Claude Opus 5 (1M context) --- .../spark/src/test/resources/log4j2.properties | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) create mode 100644 connectors/spark/src/test/resources/log4j2.properties diff --git a/connectors/spark/src/test/resources/log4j2.properties b/connectors/spark/src/test/resources/log4j2.properties new file mode 100644 index 00000000..c2b9b815 --- /dev/null +++ b/connectors/spark/src/test/resources/log4j2.properties @@ -0,0 +1,18 @@ +# Test logging for the Spark connector. +# +# Spark logs at INFO to stderr, which sbt tags [error]. In a passing CI job that +# produced 226,580 of 240,773 log lines, so a real failure was unfindable and the +# log archive ran to ~49MB. Keep the framework at WARN and Unity Catalog's own +# loggers at INFO, so test diagnostics survive. + +rootLogger.level = warn +rootLogger.appenderRef.stderr.ref = stderr + +appender.stderr.type = Console +appender.stderr.name = stderr +appender.stderr.target = SYSTEM_ERR +appender.stderr.layout.type = PatternLayout +appender.stderr.layout.pattern = %d{yy/MM/dd HH:mm:ss} %p %c{1}: %m%n + +logger.unitycatalog.name = io.unitycatalog +logger.unitycatalog.level = info From a179857099f38b702e889f486dc62455245be635 Mon Sep 17 00:00:00 2001 From: weishan <11898366+weiishann@users.noreply.github.com> Date: Sun, 20 Sep 2026 22:54:17 +0800 Subject: [PATCH 4/4] ci: let dependabot track actions, npm and docker dependencies Action pins age out quietly: the checkout and setup-java pins in use target Node 20, which the runners have already deprecated. sbt is not a supported ecosystem, so build.sbt still needs updating by hand. Co-Authored-By: Claude Opus 5 (1M context) --- .github/dependabot.yml | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) create mode 100644 .github/dependabot.yml diff --git a/.github/dependabot.yml b/.github/dependabot.yml new file mode 100644 index 00000000..bbf3fecc --- /dev/null +++ b/.github/dependabot.yml @@ -0,0 +1,29 @@ +# sbt is not a supported ecosystem, so JVM dependencies in build.sbt still need +# updating by hand. These cover the parts Dependabot can reach. + +version: 2 +updates: + # Action pins age out quietly: the checkout and setup-java pins currently in + # use target Node 20, which the runners have already deprecated. + - package-ecosystem: "github-actions" + directory: "/" + schedule: + interval: "weekly" + groups: + actions: + patterns: + - "*" + + - package-ecosystem: "npm" + directory: "/ui" + schedule: + interval: "weekly" + groups: + ui-dependencies: + patterns: + - "*" + + - package-ecosystem: "docker" + directory: "/" + schedule: + interval: "weekly"