Skip to content

ci: remove concurrency group from cpu_test workflow#248

Merged
shuheng-liu merged 1 commit into
mainfrom
claude/remove-ci-concurrency-group-M1XFL
May 4, 2026
Merged

ci: remove concurrency group from cpu_test workflow#248
shuheng-liu merged 1 commit into
mainfrom
claude/remove-ci-concurrency-group-M1XFL

Conversation

@shuheng-liu
Copy link
Copy Markdown
Member

@shuheng-liu shuheng-liu commented May 4, 2026

What this does

Removes the concurrency block from .github/workflows/cpu_test.yml. With cancel-in-progress: true, every superseded run on a PR was reported as a failed status check, making PRs look red even when the underlying tests would have passed on a fresh run.

How it was tested

No code paths changed — workflow-config-only edit. Verified the YAML still parses by running pre-commit (check yaml, zizmor, etc. all pass). CI on this PR will exercise the new behavior end-to-end.

How to checkout & try? (for the reviewer)

git fetch origin claude/remove-ci-concurrency-group-M1XFL
git checkout claude/remove-ci-concurrency-group-M1XFL
git diff main -- .github/workflows/cpu_test.yml

Checklist

  • I have added Google-style docstrings to important functions and ensured function parameters are typed.
  • My PR includes policy-related changes.
    • If the above is checked: I have run the GPU pytests (pytest -m "gpu") and regression tests.

Note: Before submitting this PR, please read the contributor guideline.


Generated by Claude Code

@shuheng-liu shuheng-liu self-assigned this May 4, 2026
@shuheng-liu shuheng-liu marked this pull request as ready for review May 4, 2026 07:14
Copy link
Copy Markdown
Contributor

@claude claude Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inline finding posted; see summary comment.

group: cpu-test-${{ github.head_ref || github.ref_name }}
cancel-in-progress: true

env:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-introduces the self-race that PR #232 explicitly fixed. cpu_test.yml triggers on both pull_request and push (lines 19-20) without branch filters, so every push to a PR branch fires two concurrent runs at the same SHA. Per #232, those duplicate runs race on shared external resources (HF Hub auth + dataset downloads) and the losing one consistently fails — burning ~8 min of CPU twice and producing the very red checks this PR is trying to avoid.

If the original concern is that cancelled runs show as failed status checks, milder fixes preserve dedup:

  • Switch cancel-in-progress: truecancel-in-progress: false (queue rather than cancel; no run is ever cancelled, so no "failed" status from cancellation).
  • Or restrict push: to branches: [main] so PR pushes only fire pull_request once, then drop the concurrency block.

Worth verifying that cancelled runs were actually showing as failed (not cancelled) before reverting #232 — branch protection treats those distinctly by default.

@claude
Copy link
Copy Markdown
Contributor

claude Bot commented May 4, 2026

[claude-review] summary for commit 2231d1d

  • blocking.github/workflows/cpu_test.yml:25 — Removing concurrency: reverts the self-race fix from PR test(pi07): drop VJEPA2 test + fix cpu_test.yml self-race (#234) #232. The workflow triggers on both pull_request and push with no branch filters, so each PR push fires two concurrent runs at the same SHA that race on shared HF Hub auth + dataset downloads (the losing one consistently fails). Consider cancel-in-progress: false (queue instead of cancel) or restricting push: to branches: [main] instead of dropping the block entirely.

@shuheng-liu shuheng-liu merged commit 15ebcc9 into main May 4, 2026
8 of 9 checks passed
@shuheng-liu shuheng-liu deleted the claude/remove-ci-concurrency-group-M1XFL branch May 4, 2026 07:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant