Await Fast CI #33241
Open
Await Fast CI #33241
Conversation
This file contains 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
Update an example build to call the await-fast-ci.yaml job which currently just echos
|
PR #33241: Size comparison from 416f6a3 to 0331c14 Full report (38 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, linux, mbed, nrfconnect, nxp, psoc6, qpg, stm32)
|
|
PR #33241: Size comparison from 416f6a3 to cf2bf37 Full report (4 builds for cc32xx, mbed, stm32)
|
|
PR #33241: Size comparison from 416f6a3 to 26da209 Decreases (2 builds for efr32)
Full report (71 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, mbed, nrfconnect, nxp, psoc6, qpg, stm32, telink)
|
yunhanw-google
approved these changes
Apr 30, 2024
yunhanw-google
requested changes
Apr 30, 2024
pullapprove
bot
requested review from
rcasallas-silabs,
ReneJosefsen,
robszewczyk,
saurabhst,
selissia,
sharadb-amazon,
tcarmelveilleux,
tecimovic,
tehampson,
tima-q,
tobiasgraf,
turon,
vivien-apple,
wiba-nordic,
woody-apple,
younghak-hwang and
yufengwangca
|
PR #33241: Size comparison from 1919112 to 601d67e Full report (85 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, mbed, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
cecille
reviewed
Jul 8, 2024
| exit(1) | ||
|
|
||
| print( | ||
| f"Polling for completion of fast CI {args.check} failed. Please ensure the name of the check was entered correctly and verify that it should take less than {poll_max} seconds to run.") |
There was a problem hiding this comment.
Does this imply that if the fast jobs don't complete within 300s that this will fail and we'll cancel all the remaining CI jobs?
I think in this case maybe we should just let the complete? Thoughts @andy31415 ?
| poll_max = 300 | ||
| while (polling): | ||
| for line in subprocess.run(f"gh pr checks -R project-chip/connectedhomeip {args.pr}", stdout=subprocess.PIPE, shell=True).stdout.decode("utf-8").splitlines(): | ||
| if args.check in line: |
There was a problem hiding this comment.
This feels fragile - you're parsing the log line, but if the log line changes and you get back something that doesn't have args.check, or "pending", "pass" or "fail" then you'll loop forever.
If you're going to use polling here, the count increment needs to be outside of the nested if.
|
@yunhanw-google - please see attached issue. The goal is to cancel the slow-running CI targets when the fast targets fail, so we don't spend multiple hours of CI time running jobs when we know the PR will need to be updated. |
And what about the case when someone would like to know all failing platforms (workflows) in advance? In case of fast failure, CI will report only the first one. Then one will fix it and learn in another x minutes that another platform (workflow) fails... I know that preferably it would be nice to test everything locally before pushing to upstream, but it's not that simple with constrained development environment (storage, RAM, CPU) and so many platforms to test. Other possibility is to test CI on local fork and then create PR. However, such approach will double the CI time in most cases (there was already a case reported by GitHub, that this project uses too much GitHub Actions resources dedicated for ALL open source projects). I hope that we will not end up in a case similar to Darwin matrix workflow were one flaky workflow stops others, so instead of rerunning one job it is required to rerun two or three. |
|
For simple cases like CLA or restyled there is already an action that can pause a job before other is completed: https://github.com/marketplace/actions/wait-on-check IMHO it's better not to spawn any jobs if restyle (or other simple check like that) does not pass. So, for such approach see https://github.com/marketplace/actions/wait-on-check#alternatives maybe one of these will be useful. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
#32735
Created as a draft to prototype functionality