Skip to content

Add a workflow to promote green commit to stable branch #1697

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 9 commits into from
Closed

Add a workflow to promote green commit to stable branch #1697

wants to merge 9 commits into from

Conversation

huydhn
Copy link
Contributor

@huydhn huydhn commented Jan 25, 2024 •
edited
Loading

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

@pytorch-bot
Copy link

pytorch-bot bot commented Jan 25, 2024 •
edited
Loading

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/1697

Note: Links to docs will display an error until the docs builds have been completed.

✅ No Failures

As of commit 0588c4b with merge base c99e5a5 (image):
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jan 25, 2024
@huydhn huydhn temporarily deployed to update-viable-strict January 25, 2024 01:31 — with GitHub Actions Inactive
@huydhn huydhn requested a review from guangy10 January 25, 2024 01:36
@huydhn huydhn marked this pull request as ready for review January 25, 2024 01:36
@huydhn
Copy link
Contributor Author

huydhn commented Jan 25, 2024

@guangy10 Let's chat more on how to get ET trunk into a good state before landing this change.

@facebook-github-bot
Copy link
Contributor

@huydhn has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@huydhn has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@huydhn
Copy link
Contributor Author

huydhn commented Jan 27, 2024

@huydhn huydhn mentioned this pull request Jan 27, 2024
Closed
guangy10
guangy10 reviewed Jan 30, 2024
View reviewed changes
.github/workflows/update-viablestrict.yml Outdated
with:
repository: pytorch/executorch
stable-branch: viable/strict
requires: '[\"pull\", \"lint\"]'
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we include doc build as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We could, we can add that if a successful doc build is considered important enough for the stable branch. Let me add that, if it turns out not needed, it can be removed.

guangy10
guangy10 approved these changes Jan 30, 2024
View reviewed changes
Copy link
Contributor

@guangy10 guangy10 left a comment

Choose a reason for hiding this comment

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

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.

It makes sense to get the process up running asap as long as all pull, lint and doc build pass. As the 2nd step to tighten the quality of viable/strict, instead of waiting for all trunk job to be green, I'm wondering if there is a way to disable failed jobs from running on trunk, e.g. mark disabled/skipped, until those jobs are fixed. My concern is that fixing all trunk jobs may take long time especially there are some jobs owned by the external partners, e.g. test-arm-reference-delegation, test-coreml-delegate. when one gets fixed, a new one may start failing.

@guangy10
Copy link
Contributor

guangy10 commented Jan 30, 2024 •
edited
Loading

fyi, #1569 is to fix the test-arm-backend-delegate. We have rooted cause the issue.

@guangy10
Copy link
Contributor

FYI, the issue #1751 is fixed in #1747. Just approved the fix, and will start merging

@huydhn huydhn temporarily deployed to update-viable-strict January 31, 2024 02:39 — with GitHub Actions Inactive
@huydhn huydhn temporarily deployed to update-viable-strict January 31, 2024 02:49 — with GitHub Actions Inactive
@huydhn huydhn temporarily deployed to update-viable-strict January 31, 2024 02:51 — with GitHub Actions Inactive
@facebook-github-bot
Copy link
Contributor

@huydhn has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@huydhn merged this pull request in d0050dd.

facebook-github-bot pushed a commit that referenced this pull request Feb 2, 2024
@huydhn @facebook-github-bot
Fix MobileBert timeout CI failures (#1729)
340c69b 
Summary:
I'm making the timeout value configurable for each models and defaults to 90m (the current value).  The timeout value for MobileBert can then be increased to ~120m~ 180m.  The model is running only trunk anyway, so there is no impact on PR duration. A better fix would be to figure out why it's running slowly, but I leave it to the experts.

I'm trying to get lint and pull signals all green before landing #1697 (although they are not that related).

Pull Request resolved: #1729

Reviewed By: mergennachin

Differential Revision: D53149238

Pulled By: huydhn

fbshipit-source-id: fdb96197a8776b957316c5b448db3fb14b9e3ed8
@guangy10 guangy10 mentioned this pull request Feb 29, 2024
Closed
guangy10 added a commit to guangy10/executorch-1 that referenced this pull request Feb 29, 2024
@guangy10 @facebook-github-bot
Restrict promoting commit to stable branch
f113675 
Summary:
Currently the commit got promoted to viable/strict isn't stable: [ET CI HUD](https://hud.pytorch.org/hud/pytorch/executorch/viable%2Fstrict).

ET CI is in a better state now, it's time to restrict promoting commit by including trunk jobs. It is a follow-up action from [pytorch#1697](pytorch#1697).

Differential Revision: D54370551
guangy10 added a commit to guangy10/executorch-1 that referenced this pull request Mar 1, 2024
@guangy10 @facebook-github-bot
Restrict promoting commit to stable branch
cb3acbb 
Summary:
Currently the commit got promoted to viable/strict isn't stable: [ET CI HUD](https://hud.pytorch.org/hud/pytorch/executorch/viable%2Fstrict).

ET CI is in a better state now, it's time to restrict promoting commit by including trunk jobs. It is a follow-up action from [pytorch#1697](pytorch#1697).

Reviewed By: huydhn

Differential Revision: D54370551
guangy10 added a commit to guangy10/executorch-1 that referenced this pull request Mar 1, 2024
@guangy10 @facebook-github-bot
Restrict promoting commit to stable branch
583c68e 
Summary:
Currently the commit got promoted to viable/strict isn't stable: [ET CI HUD](https://hud.pytorch.org/hud/pytorch/executorch/viable%2Fstrict).

ET CI is in a better state now, it's time to restrict promoting commit by including trunk jobs. It is a follow-up action from [pytorch#1697](pytorch#1697).

Reviewed By: huydhn

Differential Revision: D54370551
guangy10 added a commit to guangy10/executorch-1 that referenced this pull request Mar 1, 2024
@guangy10 @facebook-github-bot
Restrict promoting commit to stable branch
d6b41b0 
Summary:
Currently the commit got promoted to viable/strict isn't stable: [ET CI HUD](https://hud.pytorch.org/hud/pytorch/executorch/viable%2Fstrict).

ET CI is in a better state now, it's time to restrict promoting commit by including trunk jobs. It is a follow-up action from [pytorch#1697](pytorch#1697).

Reviewed By: huydhn

Differential Revision: D54370551
facebook-github-bot pushed a commit that referenced this pull request Mar 1, 2024
@guangy10 @facebook-github-bot
Restrict promoting commit to stable branch
c3c667e 
Summary:
bypass-github-export-checks

Currently the commit got promoted to viable/strict isn't stable: [ET CI HUD](https://hud.pytorch.org/hud/pytorch/executorch/viable%2Fstrict).

ET CI is in a better state now, it's time to restrict promoting commit by including trunk jobs. It is a follow-up action from [#1697](#1697).

Reviewed By: huydhn

Differential Revision: D54370551

fbshipit-source-id: dc94dc72360505ded174dcdfb90febda49a8e276
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@guangy10 guangy10 guangy10 approved these changes

Assignees

No one assigned

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@huydhn @facebook-github-bot @guangy10