-
Notifications
You must be signed in to change notification settings - Fork 107
Migrate update-viablestrict to test-infra as a generic GHA #4905
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Ignored Deployment
|
cc @guangy10 FYI, this is the GitHub action that PyTorch is using to promote a main commit to viable/strict. Having this here will allow ExecuTorch to use it too, and I can use this to create the stable branch for ET, and subsequently nightly. |
|
||
def get_latest_green_commit( | ||
commits: List[str], requires: List[str], results: List[Dict[str, Any]] | ||
) -> Any: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Would be Optional[str] better since the return type is either None or a str
# HACK: Same commit were merged, reverted and landed again | ||
# which creates a tracking problem |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Curious to ask: What is the tracking problem? Would you mind elaborating more? I think viablestrict should never set to a commit that will be reverted (because it's not green), so why is tracking the reverted/re-landed hash matters
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let me check this part from PyTorch for more historical context, the script is copied from there. One one hand, it's easier to keep the script as it is (which I think I will do). On the other hand, this definitely looks like some hacks to be removed (may be as a separate BE change).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm assuming this is pretty much just a copy paste of the one in pytorch/pytorch so it's fine.
I'm not a super big fan of the entire gitutils being copied, since I doubt updating viable/strict actually uses all of it
Yup, but then I plan to eventually move the whole merge bot / revert bot here too, so I think it's ok because gitutils on PyTorch can be removed then. That will happen in few months time I think, but we rarely change gitutils nowadays, so it would be ok. |
In pytorch/test-infra#4905, so that ExecuTorch can use the same GHA on their CI. ### Testing https://github.com/pytorch/pytorch/actions/runs/7634906738/job/20799502532#step:2:15480 Pull Request resolved: #118163 Approved by: https://github.com/clee2000
Summary: This is ready now after pytorch/test-infra#4905 lands. I reuse the same convention from PyTorch here calling the stable branch `viable/strict`. * I have already create the branch https://github.com/pytorch/executorch/tree/viable/strict as a protected one. Only repo admin or the update bot can write into it. * The update job is scheduled to run every 30m (copied from PyTorch) * A commit can be promoted to viable/strict if all its pull and lint jobs pass. The condition here can be extended to include trunk jobs too, but we need to get ET trunk into a healthy state before that. * My plan is to try to get lint and pull to a good state first, land this change, then fix trunk jobs, and add trunk signals too. ### Testing https://github.com/pytorch/executorch/actions/runs/7648614817/job/20841686469?pr=1697#step:2:1270, no green commits was found because there are broken lint and pull jobs on main https://hud.pytorch.org/hud/pytorch/executorch/main Pull Request resolved: #1697 Reviewed By: guangy10 Differential Revision: D53072054 Pulled By: huydhn fbshipit-source-id: 31dd1aec3a5c77ae4ed3639e63c4c6620340773e
This will be needed by ExecuTorch too. So, I think it's time to move this to test-infra as a generic GHA. The list includes:
update-viablestrict
GHA that could be used by PyTorch and ExecuTorch.fetch_latest_green_commit.py
script and its unit tests. I also refactor theis_green
function to accept a list of required jobs that need to be pass before marking a commit as green. Before this, the list is fixed https://github.com/pytorch/pytorch/blob/main/.github/scripts/fetch_latest_green_commit.py#L89-L94 to match what we have on PyTorch.Testing
Locally on PyTorch:
On PyTorch, pytorch/pytorch#118163 and manually dispatch the job https://github.com/pytorch/pytorch/actions/runs/7634906738/job/20799502532#step:2:15480