nevzheng commented on code in PR #12545: URL: https://github.com/apache/gravitino/pull/12545#discussion_r3905978400
########## .github/workflows/required-ci.yml: ########## @@ -0,0 +1,187 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +# + +# Aggregates the reusable CI suites into one stable `Required CI` check that a +# `main` ruleset can require, so merges are gated on CI and not on review +# alone. This parent is the sole pull-request listener; each child suite is +# invoked via `workflow_call` and keeps its own path-based skip, so a PR that +# touches only docs no-ops every suite but still reports a green `Required CI`. +# Each called workflow also appears as its own check in the PR checks panel, +# so contributors keep per-suite visibility. `push` triggers on the children +# are unchanged, so branch builds behave exactly as before. +# +# Rollout is staged: land this workflow (no ruleset yet) -> canary on a few PRs +# -> enable the `main` ruleset requiring `Required CI` (and the standalone +# `conflict-marker-check`). Fully reversible: delete this file and restore the +# children's `pull_request:` trigger. +# +# The workflow `name` is a contract: coverage-comment.yml's workflow_run +# listener matches `Required CI`. Do not rename it without updating that file. +name: Required CI + +on: + pull_request: + branches: [main, branch-*] + # Minimal event set: a new PR, a new push to an open PR, or a reopened PR. + # Excluding labeled/unlabeled/edited avoids redundant runs (upstream has + # no opt-in label; every PR runs CI). + types: [opened, synchronize, reopened] + +concurrency: + group: required-ci-${{ github.event.pull_request.number || github.ref }} + # Cancel the superseded PR run. Children already cancel in-progress; the + # parent must too or a second push waits out a full stale 14-suite run. + cancel-in-progress: true + +jobs: + build: + uses: ./.github/workflows/build.yml + with: + required_ci: true + secrets: inherit + + backend: + uses: ./.github/workflows/backend-integration-test.yml + with: + required_ci: true + secrets: inherit + + spark: + uses: ./.github/workflows/spark-integration-test.yml + with: + required_ci: true + secrets: inherit + + flink: + uses: ./.github/workflows/flink-integration-test.yml + with: + required_ci: true + secrets: inherit + + trino: + uses: ./.github/workflows/trino-integration-test.yml + with: + required_ci: true + secrets: inherit + + iceberg_rest_trino: + uses: ./.github/workflows/iceberg-rest-trino-integration-test.yml + with: + required_ci: true + secrets: inherit + + python: + uses: ./.github/workflows/python-integration-test.yml + with: + required_ci: true + secrets: inherit + + frontend: + uses: ./.github/workflows/frontend-integration-test.yml + with: + required_ci: true + secrets: inherit + + access_control: + uses: ./.github/workflows/access-control-integration-test.yml + with: + required_ci: true + secrets: inherit + + idp_basic: + uses: ./.github/workflows/idp-basic-test.yml + with: + required_ci: true + secrets: inherit + + mcp: + uses: ./.github/workflows/mcp-integration-test.yml + with: + required_ci: true + secrets: inherit + + maintenance: + uses: ./.github/workflows/maintenance-integration-test.yml + with: + required_ci: true + secrets: inherit + + contrib_catalog: + uses: ./.github/workflows/contrib-catalog-test.yml + with: + required_ci: true + secrets: inherit + + web_ui: + uses: ./.github/workflows/web-ui-tests.yml + with: + required_ci: true + secrets: inherit + + # Aggregate every reusable suite into one check. `if: always()` runs even + # when an upstream suite is skipped (path filter), because a skipped suite + # reports `success` and must not fail the aggregate. The job fails only if + # any suite reports a non-success result. + required_ci: + name: Required CI + if: always() + needs: + - build + - backend + - spark + - flink + - trino + - iceberg_rest_trino + - python + - frontend + - access_control + - idp_basic + - mcp + - maintenance + - contrib_catalog + - web_ui Review Comment: Done. `Test Charts` and `asf-allowlist-check` are reusable calls under Required CI (`charts`, `asf_allowlist`). Chart `lint-test` still skips when those paths miss; the reusable call stays success. Allowlist always runs. `conflict-marker-check` stays standalone. Implemented in 8293af98b. Nevin Sent from my 🤖 (Cursor) ########## dev/ci/test_required_ci.py: ########## @@ -0,0 +1,292 @@ +#!/usr/bin/env python3 +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +"""Check the Required CI parent/child contract without starting Actions. + +This script does not contact GitHub. It reads the checked-in workflow +sources and asserts the contract this PR relies on: + +- the parent is the sole pull_request listener for the aggregated suites +- each suite is reusable via workflow_call + required_ci and keeps push +- Required CI is a static always() aggregate that fails on any non-success +- conflict-marker-check stays standalone and is not part of the aggregate +- required-mode concurrency keys are unique per suite +- standalone (push) keys use github.workflow, not a shared 'standalone' literal +- the parent cancels superseded PR runs +- web-ui path-filters inside the called workflow +- coverage-comment listens for the Required CI parent, not a standalone build run +""" + +import re +from pathlib import Path + + +REPO_ROOT = Path(__file__).resolve().parents[2] +WORKFLOW_DIR = REPO_ROOT / ".github" / "workflows" +REQUIRED_CI_WORKFLOW = WORKFLOW_DIR / "required-ci.yml" +CONFLICT_MARKER_WORKFLOW = WORKFLOW_DIR / "conflict-marker-check.yml" +COVERAGE_COMMENT_WORKFLOW = WORKFLOW_DIR / "coverage-comment.yml" + +SUITE_WORKFLOWS = { Review Comment: Done. The contract script now discovers `*.yml` and `*.yaml`, requires every `pull_request` workflow to be this parent, an aggregated suite, or an explicit allowlist (`conflict-marker-check.yml`), and runs from Required CI. Implemented in 8293af98b. Nevin Sent from my 🤖 (Cursor) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
