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" 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 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/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 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"