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
[GitHub Actions] run manifest_build.py directly on Ubuntu 18.04 #19580
Conversation
In 192914f#r35409518 @jgraham asked:
That would indeed be possible, and might be 15-30 seconds faster than this if the docker image is cached, but the dependencies are so few/simple here that I'm not sure the extra work of maintaining/updating the image are worth it. @jugglinmike thoughts? |
Fixes #19380. Running faster increases the risk that the GitHub search API can't find the pull request yet, so add retry.py with a --delay value that with 5 iterations adds up to a total of 15 minutes waiting.
0fb3ee9
to
344fc3a
Compare
Comparing these two checks gives an idea about the speed improvement: The original setup took 218s in "Run manifest_build.py", while this PR took 94s for installing dependencies and running the same step. It didn't actually upload the assets and create the release, but that part was only 2s in the original setup. So, one can shave off 2 minutes or so. At the end of the day the speedup is not hugely important, a more direct benefit is that the logs are cleaner as they don't start with a whole lot of docker fetching. |
I am in favour, but I leave the review to @jgraham. |
Alright, https://github.com/web-platform-tests/wpt/runs/259065754 worked and was not all too slow. Now let's see how long it takes before the job fails when fetching from pythonhosted.org and what to do then... |
This is like #19580, but the dependencies of this job are even more modest and are already fulfilled by the base ubuntu-18.04 image.
In #19580 retry for manifest generation was introduced, to make it more robust in case of network errors of the GitHub search API not indexing the merged PR fast enough. However, an accidental commit to master showed that this won't work: https://github.com/web-platform-tests/wpt/runs/265191690 It first failed with "No PR found for 9e1d0a2" because there was no PR, just a direct push to master. Retries then failed with "File exists: '/home/runner/meta'". Use a new temporary directory every time to deal with this. Don't attempt to remove the directory when done as this script is run in a CI system where all state is thrown away on completion.
In #19580 retry for manifest generation was introduced, to make it more robust in case of network errors or the GitHub search API not indexing the merged PR fast enough. However, an accidental commit to master showed that this won't work: https://github.com/web-platform-tests/wpt/runs/265191690 It first failed with "No PR found for 9e1d0a2" because there was no PR, just a direct push to master. Retries then failed with "File exists: '/home/runner/meta'". Use a new temporary directory every time to deal with this. Don't attempt to remove the directory when done as this script is run in a CI system where all state is thrown away on completion.
In #19580 retry for manifest generation was introduced, to make it more robust in case of network errors or the GitHub search API not indexing the merged PR fast enough. However, an accidental commit to master showed that this won't work: https://github.com/web-platform-tests/wpt/runs/265191690 It first failed with "No PR found for 9e1d0a2" because there was no PR, just a direct push to master. Retries then failed with "File exists: '/home/runner/meta'". Use a new temporary directory every time to deal with this. Don't attempt to remove the directory when done as this script is run in a CI system where all state is thrown away on completion.
…st generation in CI, a=testonly Automatic update from web-platform-tests Use a new temporary directory for manifest generation in CI (#19802) In web-platform-tests/wpt#19580 retry for manifest generation was introduced, to make it more robust in case of network errors or the GitHub search API not indexing the merged PR fast enough. However, an accidental commit to master showed that this won't work: https://github.com/web-platform-tests/wpt/runs/265191690 It first failed with "No PR found for 9e1d0a26f155ccb088d342b0036e535d033423c1" because there was no PR, just a direct push to master. Retries then failed with "File exists: '/home/runner/meta'". Use a new temporary directory every time to deal with this. Don't attempt to remove the directory when done as this script is run in a CI system where all state is thrown away on completion. -- wpt-commits: 3b13e3a1313e08cf4bc5270ab59c149a2bc56260 wpt-pr: 19802
…st generation in CI, a=testonly Automatic update from web-platform-tests Use a new temporary directory for manifest generation in CI (#19802) In web-platform-tests/wpt#19580 retry for manifest generation was introduced, to make it more robust in case of network errors or the GitHub search API not indexing the merged PR fast enough. However, an accidental commit to master showed that this won't work: https://github.com/web-platform-tests/wpt/runs/265191690 It first failed with "No PR found for 9e1d0a26f155ccb088d342b0036e535d033423c1" because there was no PR, just a direct push to master. Retries then failed with "File exists: '/home/runner/meta'". Use a new temporary directory every time to deal with this. Don't attempt to remove the directory when done as this script is run in a CI system where all state is thrown away on completion. -- wpt-commits: 3b13e3a1313e08cf4bc5270ab59c149a2bc56260 wpt-pr: 19802
…st generation in CI, a=testonly Automatic update from web-platform-tests Use a new temporary directory for manifest generation in CI (#19802) In web-platform-tests/wpt#19580 retry for manifest generation was introduced, to make it more robust in case of network errors or the GitHub search API not indexing the merged PR fast enough. However, an accidental commit to master showed that this won't work: https://github.com/web-platform-tests/wpt/runs/265191690 It first failed with "No PR found for 9e1d0a26f155ccb088d342b0036e535d033423c1" because there was no PR, just a direct push to master. Retries then failed with "File exists: '/home/runner/meta'". Use a new temporary directory every time to deal with this. Don't attempt to remove the directory when done as this script is run in a CI system where all state is thrown away on completion. -- wpt-commits: 3b13e3a1313e08cf4bc5270ab59c149a2bc56260 wpt-pr: 19802 UltraBlame original commit: 9fb5eab0466068bf41f77823582dde61fa26ed7b
…st generation in CI, a=testonly Automatic update from web-platform-tests Use a new temporary directory for manifest generation in CI (#19802) In web-platform-tests/wpt#19580 retry for manifest generation was introduced, to make it more robust in case of network errors or the GitHub search API not indexing the merged PR fast enough. However, an accidental commit to master showed that this won't work: https://github.com/web-platform-tests/wpt/runs/265191690 It first failed with "No PR found for 9e1d0a26f155ccb088d342b0036e535d033423c1" because there was no PR, just a direct push to master. Retries then failed with "File exists: '/home/runner/meta'". Use a new temporary directory every time to deal with this. Don't attempt to remove the directory when done as this script is run in a CI system where all state is thrown away on completion. -- wpt-commits: 3b13e3a1313e08cf4bc5270ab59c149a2bc56260 wpt-pr: 19802 UltraBlame original commit: 9fb5eab0466068bf41f77823582dde61fa26ed7b
…st generation in CI, a=testonly Automatic update from web-platform-tests Use a new temporary directory for manifest generation in CI (#19802) In web-platform-tests/wpt#19580 retry for manifest generation was introduced, to make it more robust in case of network errors or the GitHub search API not indexing the merged PR fast enough. However, an accidental commit to master showed that this won't work: https://github.com/web-platform-tests/wpt/runs/265191690 It first failed with "No PR found for 9e1d0a26f155ccb088d342b0036e535d033423c1" because there was no PR, just a direct push to master. Retries then failed with "File exists: '/home/runner/meta'". Use a new temporary directory every time to deal with this. Don't attempt to remove the directory when done as this script is run in a CI system where all state is thrown away on completion. -- wpt-commits: 3b13e3a1313e08cf4bc5270ab59c149a2bc56260 wpt-pr: 19802 UltraBlame original commit: 9fb5eab0466068bf41f77823582dde61fa26ed7b
Fixes #19380.
Running faster increases the risk that the GitHub search API can't
find the pull request yet, so add retry.py with a --delay value that
with 5 iterations adds up to a total of 15 minutes waiting.