ci: Enable all functional GH workflows for pushes and pull requests; Enable workflow-level concurrency groups for efficiency. - #7
Conversation
…ows when new commits are pushed; Remove job-level concurrency groups.
This reverts commit 07eb7a4.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis set of changes standardizes and centralizes concurrency control across multiple GitHub Actions workflows by moving concurrency settings from individual jobs to the workflow level. Workflow triggers are expanded to include both pull request and push events, with branch-specific cancellation logic. One workflow file, Changes
Sequence Diagram(s)sequenceDiagram
participant Developer
participant GitHub
participant Workflow
participant Job
Developer->>GitHub: Push or Pull Request event
GitHub->>Workflow: Trigger workflow (with concurrency group)
alt If previous run in same group (not main branch)
Workflow->>GitHub: Cancel in-progress run
end
Workflow->>Job: Start job(s)
Job->>GitHub: Report status/results
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (17)
.github/workflows/arrow-flight-tests.yml(2 hunks).github/workflows/codenotify.yml(0 hunks).github/workflows/docs.yml(2 hunks).github/workflows/hive-tests.yml(3 hunks).github/workflows/kudu.yml(3 hunks).github/workflows/maven-checks.yml(2 hunks).github/workflows/prestocpp-format-and-header-check.yml(1 hunks).github/workflows/prestocpp-linux-build-and-unit-test.yml(1 hunks).github/workflows/prestocpp-linux-build.yml(1 hunks).github/workflows/prestocpp-macos-build.yml(1 hunks).github/workflows/product-tests-basic-environment.yml(3 hunks).github/workflows/product-tests-specific-environment.yml(3 hunks).github/workflows/singlestore-tests.yml(3 hunks).github/workflows/spark-integration.yml(3 hunks).github/workflows/test-other-modules.yml(3 hunks).github/workflows/tests.yml(3 hunks).github/workflows/web-ui-checks.yml(2 hunks)
💤 Files with no reviewable changes (1)
- .github/workflows/codenotify.yml
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/workflows/kudu.yml
[error] 32-32: wrong indentation: expected 6 but found 4
(indentation)
.github/workflows/web-ui-checks.yml
[error] 29-29: wrong indentation: expected 6 but found 4
(indentation)
.github/workflows/product-tests-specific-environment.yml
[error] 32-32: wrong indentation: expected 6 but found 4
(indentation)
.github/workflows/test-other-modules.yml
[error] 33-33: wrong indentation: expected 6 but found 4
(indentation)
.github/workflows/hive-tests.yml
[error] 33-33: wrong indentation: expected 6 but found 4
(indentation)
.github/workflows/product-tests-basic-environment.yml
[error] 32-32: wrong indentation: expected 6 but found 4
(indentation)
.github/workflows/tests.yml
[error] 33-33: wrong indentation: expected 6 but found 4
(indentation)
.github/workflows/spark-integration.yml
[error] 33-33: wrong indentation: expected 6 but found 4
(indentation)
⏰ Context from checks skipped due to timeout of 90000ms (20)
- GitHub Check: kudu (8.0.442)
- GitHub Check: kudu (17.0.13)
- GitHub Check: product-tests-specific-environment1 (8.0.442)
- GitHub Check: product-tests-specific-environment2 (8.0.442)
- GitHub Check: product-tests-specific-environment1 (17.0.13)
- GitHub Check: test-other-modules (8.0.442)
- GitHub Check: test (8.0.442, :presto-tests -P ci-only-tpch-distributed-queries)
- GitHub Check: spark-integration (17.0.13)
- GitHub Check: spark-integration (8.0.442)
- GitHub Check: hive-tests (17.0.13)
- GitHub Check: hive-tests (8.0.442)
- GitHub Check: hive-dockerized-tests (8.0.442)
- GitHub Check: hive-dockerized-tests (17.0.13)
- GitHub Check: prestocpp-linux-build-for-test
- GitHub Check: arrowflight-java-tests (8.0.442, :presto-base-arrow-flight)
- GitHub Check: prestocpp-linux-build-for-test
- GitHub Check: arrowflight-java-tests (17.0.13, :presto-base-arrow-flight)
- GitHub Check: maven-checks (8.0.442)
- GitHub Check: maven-checks (17.0.13)
- GitHub Check: prestocpp-linux-build-engine
🔇 Additional comments (30)
.github/workflows/prestocpp-format-and-header-check.yml (2)
9-12: Add push trigger for format and header checks
Expanding the workflow to run on bothpull_requestandpushevents ensures that formatting and header validations are applied consistently on every branch update.
14-19: Introduce workflow-level concurrency
Centralizing concurrency at the top level with${{github.workflow}}-${{github.ref}}and cancelling in-progress runs (except onmain) improves CI efficiency and replaces per-job settings. The YAML structure is correct..github/workflows/hive-tests.yml (2)
5-6: Includepushevent alongsidepull_request
Adding thepushtrigger ensures Hive tests run on direct branch updates as well as PRs, maintaining coverage for all commits.
16-22: Centralize concurrency at the workflow level
Replacing per-job concurrency blocks with a top-levelconcurrencysection keyed by workflow and ref, and cancelling non-mainruns, standardizes CI behaviour and reduces duplication..github/workflows/docs.yml (2)
7-9: Addpushtrigger for docs workflow
Expanding triggers to includepushonpresto-docs/**ensures documentation builds on every commit, not just PRs.
20-26: Implement top-level workflow concurrency
The newconcurrencyblock groups runs by${{github.workflow}}-${{github.ref}}and cancels in-flight jobs on non-mainbranches, replacing the old per-job concurrency. This aligns with repository-wide CI conventions..github/workflows/tests.yml (2)
5-6: Enablepushevents for test runs
Addingpushalongsidepull_requestensures the full test matrix executes on every branch update, not just PRs.
16-22: Standardize workflow-level concurrency
The top-levelconcurrencyblock improves CI throughput by cancelling outdated runs on non-mainrefs, removing per-job settings..github/workflows/kudu.yml (2)
5-6: Include push event in workflow triggers
This correctly expands the workflow to run on both PRs and pushes, ensuring CI coverage for direct branch updates.
15-21: Centralize workflow-level concurrency control
Moving concurrency to the top level with a group key of${{github.workflow}}-${{github.ref}}and conditional cancellation improves efficiency and supersedes individual job settings..github/workflows/web-ui-checks.yml (2)
5-6: Enable workflow on push events
Expanding triggers to includepushaligns with other CI flows and ensures updates on branches are validated immediately.
12-18: Adopt workflow-level concurrency control
The top-level concurrency block correctly replaces the old job-level settings, grouping by workflow and ref and cancelling in-progress runs except onmain..github/workflows/arrow-flight-tests.yml (2)
7-9: Add push trigger with path filters
Includingpushalongsidepull_requestwith the samepaths-ignoreensures that documentation-only changes are also skipped on direct pushes.
19-25: Consolidate concurrency at the workflow level
The global concurrency block will efficiently cancel stale runs on non-main branches. Removing per-job concurrency cleans up the configuration..github/workflows/singlestore-tests.yml (3)
5-6: Enable CI on both PRs and pushes
Addingpushtriggers ensures branch updates are validated automatically.
16-22: Shift to workflow-level concurrency
This global concurrency block correctly replaces the old job-level settings, providing cancellation semantics across non-main branches.
33-35: Verify checkout step with submodule recursion
The explicitactions/checkout@v4withsubmodules: recursivealigns with other workflows and ensures submodules are pulled correctly..github/workflows/spark-integration.yml (2)
5-6: Include push event in spark integration workflow
This expands coverage to branch pushes as well as PRs, maintaining consistency across CI pipelines.
16-22: Implement top-level concurrency
Grouping by${{github.workflow}}-${{github.ref}}and cancelling outdated runs on non-main branches improves efficiency and replaces per-job concurrency blocks..github/workflows/product-tests-specific-environment.yml (2)
5-5: Enable workflow execution on push events
Addingpush:ensures this workflow runs on both PRs and direct pushes, aligning with the CI objectives.
15-21: Centralize concurrency control at workflow level
The new top-levelconcurrencyblock groups by${{github.workflow}}-${{github.ref}}and cancels in-progress runs (except onmain), replacing per-job settings for efficiency..github/workflows/prestocpp-linux-build.yml (2)
30-55: Expand triggers to include push events with path filters
The addedpush:block mirrors thepull_request:paths, ensuring native build runs on branch pushes as intended.
56-61: Add workflow-level concurrency with branch-specific cancellation
Moving concurrency from jobs to the workflow scope standardizes cancellation across builds and skips cancelling commits onmain..github/workflows/maven-checks.yml (2)
5-5: Enable workflow execution on push events
Addingpush:ensures maven-checks run on both pull requests and branch pushes.
14-19: Centralize concurrency control at workflow level
The newconcurrencyblock groups by${{github.workflow}}-${{github.ref}}and cancels in-flight runs except onmain, replacing per-job concurrency..github/workflows/test-other-modules.yml (2)
5-5: Enable workflow execution on push events
Addingpush:expands test coverage to branch pushes in addition to PRs.
16-21: Centralize concurrency control at workflow level
The top-levelconcurrencyblock now handles cancellation for this workflow, removing the need for job-level settings..github/workflows/prestocpp-linux-build-and-unit-test.yml (2)
10-13: Enable workflow execution on push events (with path-ignore)
The addedpush:withpaths-ignore: ['presto-docs/**']ensures unit tests run on branch pushes except doc changes.
14-19: Centralize concurrency control at workflow level
Introduces a top-levelconcurrencyblock grouping by${{github.workflow}}-${{github.ref}}and skipping cancellations onmain, aligning with other workflows..github/workflows/product-tests-basic-environment.yml (1)
4-5: Added push trigger for workflow activation
The workflow now runs on both pull_request and push events, ensuring tests run for branch pushes as intended.
| - uses: "actions/checkout@v4" | ||
| with: | ||
| submodules: "recursive" |
There was a problem hiding this comment.
Fix YAML indentation for checkout step
The - uses: actions/checkout@v4 block is indented too far left, causing a YAML parsing error. Please apply this diff to align it under steps::
- - uses: actions/checkout@v4
- with:
- submodules: "recursive"
+ - uses: actions/checkout@v4
+ with:
+ submodules: recursive📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - uses: "actions/checkout@v4" | |
| with: | |
| submodules: "recursive" | |
| - uses: actions/checkout@v4 | |
| with: | |
| submodules: recursive |
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 33-33: wrong indentation: expected 6 but found 4
(indentation)
🤖 Prompt for AI Agents
In .github/workflows/hive-tests.yml around lines 33 to 35, the `- uses:
actions/checkout@v4` step is incorrectly indented, causing a YAML parsing error.
Adjust the indentation so that this step is properly nested under the `steps:`
key, aligning it with other steps in the workflow to ensure valid YAML
structure.
There was a problem hiding this comment.
No need to do this, because we are extending the existing github worflows from OSS presto. If you see the L26~L28 the - uses has the same indentation.
There was a problem hiding this comment.
@anlowee You're absolutely right! I can see that the existing - uses: dorny/paths-filter@v2 step on line 26 follows the same indentation pattern. Maintaining consistency with the OSS Presto workflow formatting makes perfect sense. Thank you for the clarification.
✏️ Learnings added
Learnt from: anlowee
PR: y-scope/presto#7
File: .github/workflows/hive-tests.yml:33-35
Timestamp: 2025-06-04T14:50:43.523Z
Learning: In the y-scope/presto repository, GitHub workflows are extended from OSS Presto and should maintain consistent formatting with the upstream patterns, including YAML indentation styles for workflow steps.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
| - uses: "actions/checkout@v4" | ||
| with: | ||
| submodules: "recursive" |
There was a problem hiding this comment.
Correct indentation for checkout in changes job
The checkout step’s hyphen is misaligned, triggering a YAML lint error. Please apply this diff:
- - uses: actions/checkout@v4
- with:
- submodules: "recursive"
+ - uses: actions/checkout@v4
+ with:
+ submodules: recursiveCommittable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 33-33: wrong indentation: expected 6 but found 4
(indentation)
🤖 Prompt for AI Agents
In .github/workflows/tests.yml around lines 33 to 35, the hyphen for the
checkout step is misaligned causing a YAML lint error. Fix this by properly
indenting the hyphen so it aligns correctly with other steps in the changes job,
ensuring valid YAML syntax.
| - uses: "actions/checkout@v4" | ||
| with: | ||
| submodules: "recursive" |
There was a problem hiding this comment.
Fix indentation for the checkout step
YAMLlint reports incorrect indentation here; the - uses step and its with block should be indented two spaces further under steps:.
Apply this diff:
- - uses: "actions/checkout@v4"
- with:
- submodules: "recursive"
+ - uses: "actions/checkout@v4"
+ with:
+ submodules: "recursive"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - uses: "actions/checkout@v4" | |
| with: | |
| submodules: "recursive" | |
| - uses: "actions/checkout@v4" | |
| with: | |
| submodules: "recursive" |
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 32-32: wrong indentation: expected 6 but found 4
(indentation)
🤖 Prompt for AI Agents
In .github/workflows/kudu.yml around lines 32 to 34, the checkout step is
incorrectly indented under the steps section. Increase the indentation of the `-
uses: "actions/checkout@v4"` line and its nested `with:` block by two spaces so
they are properly nested under the `steps:` key, ensuring valid YAML structure
and passing YAMLlint checks.
| - uses: "actions/checkout@v4" | ||
| with: | ||
| submodules: "recursive" |
There was a problem hiding this comment.
Correct indentation of the checkout step
YAMLlint indicates wrong indentation. The - uses and its with block need two more spaces under the steps: key.
Use this patch:
- - uses: "actions/checkout@v4"
- with:
- submodules: "recursive"
+ - uses: "actions/checkout@v4"
+ with:
+ submodules: "recursive"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - uses: "actions/checkout@v4" | |
| with: | |
| submodules: "recursive" | |
| - uses: "actions/checkout@v4" | |
| with: | |
| submodules: "recursive" |
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 29-29: wrong indentation: expected 6 but found 4
(indentation)
🤖 Prompt for AI Agents
In .github/workflows/web-ui-checks.yml around lines 29 to 31, the checkout step
is incorrectly indented under the steps key. Adjust the indentation by adding
two more spaces before the `- uses` line and its `with` block so that they are
properly nested under the `steps:` key, ensuring valid YAML structure.
2 participants
Description
This PR enables all GH workflows to run whenever there's a push or a pull-request to a branch. Unfortunately, the prestocpp-macos-build workflow is broken (likely due to using an incorrect set up for Velox), so that one has been disabled entirely. In addition, the codenotify workflow has been deleted since we don't use sourcegraph.
In addition, this PR uses workflow-level concurrency groups so that in-progress jobs are cancelled when a new push is made to a branch. As a result, any job-level concurrency groups have been removed since they're superseded by the workflow-level concurrency groups.
Checklist
breaking change.
Validation performed
Summary by CodeRabbit