feat(ci): Add advance velox actions#27505
Draft
unidevel wants to merge 1 commit intoprestodb:masterfrom
Draft
Conversation
Contributor
Reviewer's GuideAdds a new GitHub Actions workflow, "Advance velox", that automatically or manually advances the Velox submodule, manages a dedicated velox-update branch and PR, and monitors/report CI status for that PR, including notifications to a Velox team mention group. Sequence diagram for the Advance velox workflow lifecyclesequenceDiagram
actor Developer
participant Scheduler
participant GithubActions as Github_Actions_Advance_velox
participant PrestoRepo as Presto_Repository
participant VeloxRepo as Velox_Repository
participant PRs as Pull_Requests
participant CI as CI_Workflows
participant VeloxTeam as Velox_Team
alt Scheduled_run
Scheduler->>GithubActions: Trigger_workflow(event_name=schedule)
else Manual_run
Developer->>GithubActions: Trigger_workflow_dispatch(force_advance)
end
GithubActions->>PrestoRepo: Checkout(BASE_BRANCH, submodules_recursive)
GithubActions->>PrestoRepo: Check_for_existing_PR(head=velox_update_branch, base=BASE_BRANCH)
GithubActions->>VeloxRepo: Determine_current_and_latest_Velox_commits
alt PR_exists_and_new_changes_and_not_FORCE_ADVANCE
GithubActions->>PRs: Comment_new_changes_available
GithubActions->>PRs: Determine_PR_for_CI_monitoring
else No_PR_or_FORCE_ADVANCE_and_new_changes
GithubActions->>PrestoRepo: Update_Velox_submodule_to_latest
GithubActions->>PrestoRepo: Commit_and_push_to_velox_update_branch
GithubActions->>PRs: Create_or_update_PR
else No_new_changes
GithubActions-->>Developer: Summary_Velox_up_to_date
end
opt PR_for_monitoring_available
GithubActions->>PRs: Wait_for_CI_checks_to_start
PRs->>CI: Trigger_CI_workflows
loop Until_success_failure_or_timeout
GithubActions->>PRs: Query_PR_checks_status
PRs-->>GithubActions: Checks_states
end
alt CI_success
GithubActions->>PRs: Comment_CI_passed
GithubActions->>VeloxTeam: Mention_ready_for_review
else CI_failure
GithubActions->>PRs: Comment_CI_failed
GithubActions->>VeloxTeam: Mention_review_failures
else CI_timeout
GithubActions->>PRs: Comment_CI_timeout
GithubActions->>VeloxTeam: Mention_manual_check_needed
end
GithubActions-->>Developer: Write_workflow_summary
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
unidevel
force-pushed
the
advance-velox
branch
2 times, most recently
from
c29efac to
19cb438
Compare
Contributor
Author
|
@sourcery-ai review |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- The CI monitoring loop is configured for up to 5 hours (600 * 30 seconds) but the log and PR messages say it times out after 30 minutes, so the timeout messaging should be updated or the loop limits adjusted to match the stated duration.
- Consider adding a concurrency group for this workflow (e.g., keyed by the Velox update branch) to prevent overlapping scheduled and manual runs from racing on the same branch and PR.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The CI monitoring loop is configured for up to 5 hours (600 * 30 seconds) but the log and PR messages say it times out after 30 minutes, so the timeout messaging should be updated or the loop limits adjusted to match the stated duration.
- Consider adding a concurrency group for this workflow (e.g., keyed by the Velox update branch) to prevent overlapping scheduled and manual runs from racing on the same branch and PR.
## Individual Comments
### Comment 1
<location path=".github/workflows/velox-submodule-update.yml" line_range="34" />
<code_context>
+ submodules: recursive
+ token: ${{ secrets.GITHUB_TOKEN }}
+ ref: ${{ env.BASE_BRANCH }}
+ persist-credentials: false
+
+ - name: Configure Git
</code_context>
<issue_to_address>
**issue (bug_risk):** Pushing the branch is likely to fail because Git credentials are not persisted after checkout.
Because `actions/checkout` is run with `persist-credentials: false`, the token used for checkout is removed and the later `git push -f origin "$VELOX_UPDATE_BRANCH"` will run without auth. To fix this, either remove `persist-credentials: false` so credentials remain configured, or explicitly set the remote with the workflow token before pushing, e.g.
```bash
git remote set-url origin "https://x-access-token:${GITHUB_TOKEN}@github.com/${GITHUB_REPOSITORY}.git"
```
</issue_to_address>
### Comment 2
<location path=".github/workflows/velox-submodule-update.yml" line_range="277-286" />
<code_context>
+ MAX_ATTEMPTS=600 # Wait up to 5 hours (600 * 30 seconds)
</code_context>
<issue_to_address>
**nitpick:** CI timeout messages are inconsistent with the actual wait time (5 hours vs 30 minutes).
The loop runs for up to 600 × 30s = 5 hours, but the final log says:
```bash
echo "CI monitoring timed out after 30 minutes"
```
This inconsistency can mislead anyone reading the logs.
Please update the timeout message (and the notification body that uses similar wording) to match the actual 5-hour window, or reduce `MAX_ATTEMPTS` if you intend a 30-minute timeout instead.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
unidevel
force-pushed
the
advance-velox
branch
3 times, most recently
from
49978fa to
2cde290
Compare
Contributor
Author
|
@sourcery-ai review |
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The CI monitoring loop waits up to 5 hours in a single job (
MAX_ATTEMPTS=600with 30-second sleeps), which can consume a lot of runner time and approach GitHub’s max job duration; consider shortening the timeout and/or splitting monitoring into a separate, re-runnable workflow or job usingworkflow_runor scheduled polling. - The workflow logic for
FORCE_ADVANCEand PR existence is fairly intricate, with multiple conditionals controlling when branches are updated vs. only monitored; adding small helper scripts or composite actions to encapsulate the "check PR", "update submodule & branch", and "monitor CI" phases would reduce duplication and make the control flow easier to reason about.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The CI monitoring loop waits up to 5 hours in a single job (`MAX_ATTEMPTS=600` with 30-second sleeps), which can consume a lot of runner time and approach GitHub’s max job duration; consider shortening the timeout and/or splitting monitoring into a separate, re-runnable workflow or job using `workflow_run` or scheduled polling.
- The workflow logic for `FORCE_ADVANCE` and PR existence is fairly intricate, with multiple conditionals controlling when branches are updated vs. only monitored; adding small helper scripts or composite actions to encapsulate the "check PR", "update submodule & branch", and "monitor CI" phases would reduce duplication and make the control flow easier to reason about.
## Individual Comments
### Comment 1
<location path=".github/workflows/velox-submodule-update.yml" line_range="302-305" />
<code_context>
+ # Get all states
+ STATES=$(echo "$CHECKS_JSON" | jq -r '.[].state' | sort -u)
+
+ # Check if all checks are completed (no PENDING, QUEUED, or IN_PROGRESS)
+ if ! echo "$STATES" | grep -qE "PENDING|QUEUED|IN_PROGRESS"; then
+ # All checks completed, check for failures
+ if echo "$STATES" | grep -qE "FAILURE|CANCELLED|TIMED_OUT|ERROR"; then
+ echo "ci_result=failure" >> $GITHUB_OUTPUT
+ echo "CI checks failed"
</code_context>
<issue_to_address>
**issue:** Handle neutral or skipped CI states to avoid timing out when no job is actually running.
Currently, the loop only terminates on SUCCESS or explicit failure states. If all checks end up SKIPPED/NEUTRAL (or other non-running states that don’t match these patterns), the loop never exits and we hit the 5‑hour timeout. Please treat a set of only non-running, non-success, non-failure states (e.g., all SKIPPED) as a terminal condition and set `ci_result` accordingly (success or a dedicated status) instead of timing out.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
unidevel
force-pushed
the
advance-velox
branch
2 times, most recently
from
b14ccd9 to
04faea9
Compare
unidevel
changed the title
czentgr
reviewed
Apr 5, 2026
| # If force_advance is true, always use base branch (treat as new PR) | ||
| # If PR exists and not forcing, use commit from PR branch | ||
| # Otherwise use base branch | ||
| if [ "$FORCE_ADVANCE" = "true" ]; then |
Contributor
There was a problem hiding this comment.
Maybe we can simply this?
If the PR doesn't exist or we are forcing we use
CURRENT_COMMIT=$(git ls-tree "origin/$VELOX_UPDATE_BRANCH" presto-native-execution/velox | awk '{print $3}')
else we use existing commit
CURRENT_COMMIT=$(git ls-tree "origin/$VELOX_UPDATE_BRANCH" presto-native-execution/velox | awk '{print $3}')
Could we simplify this into a single if-else? Aka we are not forcing and PR exists in the if?
| - name: Update Velox submodule | ||
| if: (steps.check_pr.outputs.pr_exists != 'true' || env.FORCE_ADVANCE == 'true') && steps.check_new_changes.outputs.new_changes_available == 'true' | ||
| run: | | ||
| git -C presto-native-execution/velox checkout main |
Contributor
There was a problem hiding this comment.
Let's use git switch main (aka use the newer git API for handling the branches.
| git remote set-url origin "https://x-access-token:${GITHUB_TOKEN}@github.com/${GITHUB_REPOSITORY}.git" | ||
|
|
||
| # Ensure we're on the base branch | ||
| git checkout "$BASE_BRANCH" |
| # Ensure we're on the base branch | ||
| git checkout "$BASE_BRANCH" | ||
| # Create and switch to new branch (or switch if it exists) | ||
| git checkout -B "$VELOX_UPDATE_BRANCH" |
Contributor
There was a problem hiding this comment.
git switch -C "$VELOX_UPDATE_BRANCH"
(also updates the commit).
| # Stage the submodule changes | ||
| git add presto-native-execution/velox | ||
| # Commit the changes | ||
| git commit -m "Update Velox submodule to $VELOX_COMMIT" |
Contributor
There was a problem hiding this comment.
Lets match the commit message with the PR abstract below.
"chore(ci): Advance velox ($(date +'%Y-%m-%d'))"
| if [ -n "$CHECK_PR_NUMBER" ] || [ -n "$CREATE_PR_NUMBER" ]; then | ||
| echo "should_monitor=true" >> $GITHUB_OUTPUT | ||
| else | ||
| echo "should_monitor=false" >> $GITHUB_OUTPUT |
Contributor
There was a problem hiding this comment.
I see pr_url being used in the summary to add it to the PR description, so the pr-check job can pick it up.
But I don't see should_monitor being put into the description.
Contributor
Author
|
@czentgr Thanks for your review. I’d like to take some time to go through the flow and refactor it. I’ll let you know once it’s ready for review. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Added 2 github actions to create PR and monitor the CI status.
1. velox-advance-pr-create.yml
Creates velox advance PR.
Triggers
workflow_dispatchManual Run Modes
Default Mode (
force_advance: false)Force Update Mode (
force_advance: true)Behavior Details
masteras the base branch@prestodb/team-veloxwhen new changes are available on existing PR2. velox-advance-pr-check.yml
Checks PR CI status and notifies the team.
Triggers
test,prestocpp-linux-build, orprestocpp-linux-build-and-unit-testworkflows complete onvelox-updatebranchworkflow_dispatchBehavior
velox-updatebranch@prestodb/team-veloxwhen:Notification Group
Both workflows use the
NOTIFICATION_GROUPenvironment variable:@prestodb/team-veloxVELOX_UPDATE_NOTIFICATION_GROUPMotivation and Context
#24976
Impact
CI
Test Plan
Test in progress
Contributor checklist
Release Notes
Please follow release notes guidelines and fill in the release notes below.
Summary by Sourcery
Add a GitHub Actions workflow to automatically advance the Velox submodule and manage its update PR lifecycle.
New Features:
Enhancements:
CI:
Summary by Sourcery
Add a GitHub Actions workflow to automatically advance the Velox submodule and manage its update PR lifecycle.
New Features:
Enhancements:
CI: