Fix pull docs build running with a schedule and increase cpp doc timeout to 4h #88589
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/88589
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit 59fe49d: This comment was automatically generated by Dr. CI and updates every 15 minutes. |
huydhn
changed the title
huydhn
changed the title
| # Nightly cpp docs take about 150m to finish, and the number is stable | ||
| timeout-minutes: 180 | ||
| # TODO: Nightly cpp docs take longer and longer to finish (more than 3h now) | ||
| # Let's try to figure out how this can be improved |
| timeout-minutes: ${{ matrix.timeout-minutes }} | ||
| id: build-docs | ||
| env: | ||
| WITH_PUSH: ${{ github.event_name == 'schedule' || startsWith(github.event.ref, 'refs/tags/v') }} |
There was a problem hiding this comment.
What was the old logic here trying to do? It seems like it's only trying to push a PR when a scheduled (nightly) job is running or when a release branch (with the v tag) is running
The new behavior seeeeems to just push docs on nightlies. Have we lost the push on release branches or does that still happen?
There was a problem hiding this comment.
Hmm, why do we want to change that? I.e. this workflow is run periodically there are no harm in pushing the update, is there?
There was a problem hiding this comment.
I think the decision to push the doc or not should depend only on the inputs.push parameter. This change actually makes the workflow consistent with the logic on https://github.com/pytorch/pytorch/blob/master/.github/workflows/_docs.yml#L85. The necessary credential to push doc will only be available when inputs.push is set to true.
So pushing to a release branch or running pull as a periodic job can both publish the doc as long as the upstream workflow set the inputs.push param to true accordingly. So, even though it looks like some conditions are removed, it would make the workflow work as expected.
|
@pytorchbot merge |
1 similar comment
|
@pytorchbot merge |
Merge startedYour change will be merged once all checks pass (ETA 0-4 Hours). Learn more about merging in the wiki. Questions? Feedback? Please reach out to the PyTorch DevX Team |
These names are now available after pytorch/pytorch#88589
These names are now available after pytorch/pytorch#88589. The old names were generated from the build matrix, i.e. `docs push / build-docs (cpp, linux.12xlarge, 180)`, and required updating whenever parameters like runner type or timeout value changed. The new names are fixed at `build-docs-${{ DOC_TYPE }}-${{ PUSHED }}`. The `PUSHED` parameter will always be true here because built docs are pushed to GitHub ### Testing https://torchci-git-fork-huydhn-use-fixed-name-for-ce09c0-fbopensource.vercel.app/metrics till https://hud.pytorch.org/pytorch/pytorch/commit/c77368d41615835e5124affe79f88feed93e8855 finishes
These names are now available after pytorch/pytorch#88589. The old names were generated from the build matrix, i.e. `docs push / build-docs (cpp, linux.12xlarge, 180)`, and required updating whenever parameters like runner type or timeout value changed. The new names are fixed at `build-docs-${{ DOC_TYPE }}-${{ PUSHED }}`. The `PUSHED` parameter will always be true here because built docs are pushed to GitHub ### Testing https://torchci-git-fork-huydhn-use-fixed-name-for-ce09c0-fbopensource.vercel.app/metrics till https://hud.pytorch.org/pytorch/pytorch/commit/c77368d41615835e5124affe79f88feed93e8855 finishes
…out to 4h (pytorch#88589) * After pytorch#88373, pull workflow can now be triggered with a schedule. This changes the assumption in the doc build workflow when schedule event is used to determine if the docs should be pushed * I'll create a follow-up issue to see if it's possible to improve the performance of cpp doc build job. At the moment, it uses a linux.12xlarge runner and still couldn't finish the job after 3h Pull Request resolved: pytorch#88589 Approved by: https://github.com/seemethere, https://github.com/ZainRizvi
5 participants
Uh oh!
There was an error while loading. Please reload this page.