Conversation
|
The PR title is kind of broken, it will be nice to make it concise. |
bfcf501 to
856b5cb
Compare
|
@chenlica or @Yicong-Huang, please rerun the workflow. I removed the safe-to-test label. |
…exera into ci/AutomaticWorkflowExecution
|
I'm not sure I follow how this works now. The pr description sounds like it require a committer to add the safe to run label manually. It's still an additional step. It will be good if we can trigger CIs on PRs from anyone/ any fork, no matter if that is from committer or not, and no need for committer to manually approve it. |
|
@Yicong-Huang, I've updated the PR description and removed the The original intent was to automatically run the GitHub workflow on any PR that didn't modify the workflow itself, with the The updated design takes a simpler, safer approach: if the GitHub workflow has been modified, it will not run automatically and will require explicit committer review and approval before execution. |
make sense. since we are running actions in our main repo, it is a good idea to let committer approve altered action/workflows.
How did you verify though? |
|
I checked by raising a PR in my local fork, and it worked. Let me look and submit a fix. |
sg! It will be nice to include the link to your test PR in your fork, in the PR description. That way we have some reference of what's being tested. Thanks! |
|
If you rerun the build workflow, it should work, as I have edited the workflow; it will fail until a committer initiates the GitHub workflow. There should be a button to rerun the workflow, or you might need to go into the actions tab and run the build workflow on this pr manually. |
|
@Yicong-Huang, the current workflow fails because I triggered it and edited the build workflow. After you run the workflow, it should pass. |
Hmm, how do I run it? I don't think there is a button.. I could manually go to actions page to dispatch it, but it would be very inconvenient. |
|
I updated the code so that four events can trigger the workflow: a push to main or ci-enable/**, a pull request being opened/updated, a /safe-to-test comment on a PR, or a manual workflow_dispatch. The check-permissions job runs first in all cases. For push and dispatch, it passes straight through. For a pull request, it checks if the build workflow file was modified; if not, it passes; if yes, it checks whether the actor is a committer and fails with an error message directing them to comment /safe-to-test if not. For a /safe-to-test comment, it verifies the commenter is a committer and fails if not. Once check-permissions passes, the frontend, Scala, and Python jobs all run against the correct PR. |
|
I do not think the safe-to-test feature will work till merged into main. @Yicong-Huang, please try to re-run the workflow to test. |
|
/safe-to-test |
|
@Yicong-Huang This is a chicken-and-egg problem: a committer needs to raise the PR, or the workflows won't run. I had @aicam raise a pr of this branch. Please refer to this pr (#4287) from now on. |
|
After this is merged, commenting |
|
@Yicong-Huang, please review this. This is the first PR to handle automatic workflow execution, so committers don't have to approve every run. |
|
thanks will check later today |
|
Ok. First, it might be better to open a new Pr instead of changing the existing one, as the context can be very confusing now. So for the new problem, that we want to allow all PRs (even from forks of non-committers) can trigger CIs without explicit approval from committers. Can you explain how your change fix the issue? To me the only useful line change is to add types. What's the logic behind the fix? For the context, all PRs from committers could automatically trigger CIs today. I don't understanding why adding more event types can help non committers trigger CI without approval? |


What changes were proposed in this PR?
The build workflow now runs automatically for all PRs. All three build jobs (frontend, scala, python) trigger directly on push, pull_request, and workflow_dispatch events, making CI faster and easier to reason about for all contributors.
Any related issues, documentation, discussions?
Closes: #4290
How was this PR tested?
Verified that all three build jobs (frontend, scala, python) trigger automatically on pull request open, push, and synchronize events.
Was this PR authored or co-authored using generative AI tooling?
Resolved comments with Claude Sonnet 4.6