chore: Simplify publish_docs workflow #749
Conversation
(note: this is a re-submit of #747 to resolve CI issues.) from [janbrasna](https://github.com/janbrasna) The current docs build takes several minutes, the toolchain setup doesn't really do anything, and the workflow is triggered from unexpected events (probably not doing what it was believed to do) — so this is an attempt to simplify, and speed up. The main issues are: the curl doesn't run due to -y error; OTOH neither rustup nor cargo needs any manual setting up here… the on:check_run doesn't imply successful build, but more importantly triggers also outside of master/PRs basically on any branch anywhere commit checks are run, publishing docs on pretty much any push to any ref unnecessarily. something in the build of validator_derive (either validator, grpcio or google-cloud-rust-raw) takes ~6-7mins to build, basically 99% of the whole workflow run, so question is whether to switch to binaries or caching? The used Node 16 actions raise deprecation warnings, need updating. Logs: before: runs/10327553562 (triggered from RRM181 PR check that hasn't landed yet) after: job/28971156482#step:6:1 after cache: job/28972053092#step:6:1 More detailed explainers are in the changeset:
| - uses: actions/checkout@v3 | ||
| - name: Configure rust # TODO: can we export building rust and installing mdbook as an artifact and reuse it? | ||
| run: | | ||
| curl --proto '=https' --tlsv1.2 https://sh.rustup.rs -sSf -y | sh |
There was a problem hiding this comment.
See comment
cargo is now a part of the distribution core and we no longer need to import it directly.
There was a problem hiding this comment.
Should we still possibly run rustup update? There may be a variance between the distro version and what's currently available.
There was a problem hiding this comment.
it's kept uptodate in the runners #747 (comment) ~ and trying to update it manually only triggers warnings #747 (comment) because rustup isn't managing parts of the toolchain: #step:3:16
(similar to installing cargo audit elsewhere janbrasna#step:3:10 … as long as it's in the env setup, it's unnecessarily noisy in the logs then trying to install it manually, IMO better to skip and rely on the env for sanity;)…)
| # or run if pushes to these files. | ||
| # paths: | ||
| # - 'docs/**.md' | ||
| check_run: |
| group: "pages" | ||
| cancel-in-progress: true | ||
| group: github-pages | ||
| cancel-in-progress: false # Skip any intermediate builds but finish deploying |
There was a problem hiding this comment.
Just a question: do we for sure want a successful deploy should something in docs build fail? I wonder if this would result in us missing failed doc builds. However, it's not critical to the service health so I get the rationale here.
There was a problem hiding this comment.
If this is related to the concurrency lines directly, then they don't change anything about success of the builds. They only manage how many docs workflow runs can queue in succession, always skipping anything between the first and the last run for this concurrency group. It only changes one thing: don't cancel the one already running and wait for it to finish before starting the most recent.
(the rationale is: you never know if the newer build will be {more} successful, so chances are if the running job can deploy, let it deploy on success… in case the newer run won't build etc.)
This won't change any failed builds behaviour at all. These still fail loudly, and notify accordingly #747 (comment) — and won't deploy if the build fails.
| - uses: actions/checkout@v3 | ||
| - name: Configure rust # TODO: can we export building rust and installing mdbook as an artifact and reuse it? | ||
| run: | | ||
| curl --proto '=https' --tlsv1.2 https://sh.rustup.rs -sSf -y | sh |
There was a problem hiding this comment.
Should we still possibly run rustup update? There may be a variance between the distro version and what's currently available.
(note: this is a re-submit of #747 to resolve CI issues.)
from janbrasna
The current docs build takes several minutes, the toolchain setup doesn't really do anything, and the workflow is triggered from unexpected events (probably not doing what it was believed to do) — so this is an attempt to simplify, and speed up. The main issues are:
needs any manual setting up here…
the on:check_run doesn't imply successful build, but more
importantly triggers also outside of master/PRs basically on any branch anywhere commit checks are run, publishing docs on pretty much any push to any ref unnecessarily.
something in the build of validator_derive (either validator, grpcio
or google-cloud-rust-raw) takes ~6-7mins to build, basically 99% of the whole workflow run, so question is whether to switch to binaries or caching?
The used Node 16 actions raise deprecation warnings, need updating.
Logs:
landed yet)
after: job/28971156482#step:6:1
after cache: job/28972053092#step:6:1
More detailed explainers are in the changeset.