Migrate pytorch_python_doc_build to github action #57371

zhouzhuojie · 2021-04-30T17:44:49Z

Changes

This PR migrates pytorch_python_doc_build from circleci to github actions.

Noticeable changes

Refactor docker cp into a single docker run with volume mount, because the in circleci volume is not accessible from its remote docker engine
pytorch_python_doc_push job will have a race condition with circleci, which will be migrated in separate PRs

facebook-github-bot · 2021-04-30T17:44:57Z

💊 CI failures summary and remediations

As of commit 4c5fe73 (more details on the Dr. CI page):

💚 💚 Looks good so far! There are no failures yet. 💚 💚

This comment was automatically generated by Dr. CI (expand for details).

Follow this link to opt-out of these comments for your Pull Requests.

Please report bugs/suggestions to the (internal) Dr. CI Users group.

Click here to manually regenerate this comment.

codecov · 2021-05-03T07:55:51Z

Codecov Report

Merging #57371 (4c5fe73) into master (d212bf1) will increase coverage by 0.00%.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master   #57371   +/-   ##
=======================================
  Coverage   76.83%   76.83%           
=======================================
  Files        1986     1986           
  Lines      197988   197988           
=======================================
+ Hits       152115   152131   +16     
+ Misses      45873    45857   -16

.github/workflows/doc_build.yml

zhouzhuojie · 2021-05-03T18:13:49Z

.github/workflows/pytorch-linux-xenial-py3.6-gcc5.4.yml

here's a job if condition that doc_push only runs on master

zhouzhuojie · 2021-05-03T18:15:23Z

@seemethere updated, and moved to the same workflow as the previous python CI workflow so that they can share the artifacts. May need your approval again for the new workflows

zhouzhuojie · 2021-05-03T18:21:39Z

Oh, there's a template to generate this, hold on, will move the code there

zhouzhuojie · 2021-05-03T18:47:53Z

Oh, there's a template to generate this, hold on, will move the code there

Migrated to linux_ci_workflow.yml.in, and decouples doc build from tightly coupled with pytorch-linux-xenial-py3.6-gcc5.4, and instead just make it as a template rendering option enable_doc_jobs

zhouzhuojie · 2021-05-03T18:48:49Z

.github/templates/linux_ci_workflow.yml.in

we can also move cpp docs job to here in the future

.github/templates/linux_ci_workflow.yml.in

zhouzhuojie · 2021-05-04T18:05:45Z

.github/workflows/pytorch-linux-xenial-py3.6-gcc5.4.yml

Note that the secrets GITHUB_PYTORCHBOT_TOKEN needs to be set up first before merging

samestep · 2021-05-04T22:08:02Z

question: why is the docs build being added as a gated job inside the .github/templates/linux_ci_workflow.yml.in file, rather than as its own new template? I guess it's just because it depends on having the build already done?

(cc @seemethere)

samestep · 2021-05-04T22:09:14Z

also, I have enabled GitHub Actions to run on this PR; when you have the chance, could you fix the lints? https://github.com/pytorch/pytorch/runs/2504623867

see this section in CONTRIBUTING.md for tips on how to more easily run the lints locally; in this case, you could probably fix the trailing whitespace one by dedenting the {% if enable_doc_jobs %} line, and the trailing newlines one (currently hidden by the ShellCheck failure, but i'm guessing it would be failing) by maybe just adding a YAML comment to the end of the .github/templates/linux_ci_workflow.yml.in file

samestep · 2021-05-04T22:22:24Z

currently hidden by the ShellCheck failure, but i'm guessing it would be failing

@pytorch/pytorch-dev-infra I am currently working on #57572 which should resolve this ergonomics issue

zhouzhuojie · 2021-05-04T22:57:09Z

question: why is the docs build being added as a gated job inside the .github/templates/linux_ci_workflow.yml.in file, rather than as its own new template? I guess it's just because it depends on having the build already done?

(cc @seemethere)

Originally, I was implementing it in its own workflow, later I was debugging it and I agreed with @seemethere's comment that we can put the docs build together with linux_ci_workflow.yml.in, maybe they can share more steps together.

But, the only dependencies that the doc build job requires is a calculate-docker-image to figure out the target docker image, other build artifacts like the built docs repo can be encapsulated into the dedicated docs related workflow.

I can refactor this and make it cleaner.

zhouzhuojie · 2021-05-05T00:59:26Z

.github/workflows/docs.yml

TODO, this may also need to include the cases for

- refs/heads/master - refs/tags/vx.y.z - refs/tags/vx.y.z-rc

please add a todo comment

# TODO: add tags doc push for regex: `v[0-9]+(\.[0-9]+)*-rc[0-9]+`

also currently doc push is only enabled on nightly instead of master, should we change this to nightly?

samestep · 2021-05-05T02:09:36Z

Originally, I was implementing it in its own workflow, later I was debugging it and I agreed with @seemethere's comment that we can put the docs build together with linux_ci_workflow.yml.in, maybe they can share more steps together.

makes sense; thanks!

But, the only dependencies that the doc build job requires is a calculate-docker-image to figure out the target docker image, other build artifacts like the built docs repo can be encapsulated into the dedicated docs related workflow.

either way sounds good to me :)

seemethere · 2021-05-04T17:05:25Z

.github/templates/linux_ci_workflow.yml.in

This needs to be dependent on the build stage since building the docs requires a binary, you can reference the test stage to see how to download the artifacts from the build stage

thanks, yes sure, I may need to figure out how to iterate this faster

every change requires approvals - so each iteration requires a few hours of wait - anything special I can do to test the flow e2e on my own before requiring your review time?

yeah this is unfortunately a pain point that we haven't yet solved; one hacky solution we could take in the short-term is have you make a tiny cosmetic diff and Jedi-land it so that GitHub no longer thinks you're a "first-time contributor", after which (I hope) you wouldn't need approval to run CI on this PR; @seemethere what do you think?

as for running locally, I don't think we have a good solution for that yet either (but I could be wrong)

tested a few times, and yes, indeed it requires the artifacts generated from the build step, I updated and may need a new approval for the workflows.

Noticeable changes are

refactor docker cp into a single docker run

download artifacts from build step and mount as a volume

change from sh to bash to run in docker as it requires pushd which is a bash-only command

refactor circleci tag into GITHUB_REF - as gha only supports git ref

To pass the "first-time contributor" PR #57779

seemethere · 2021-05-04T17:06:41Z

.github/templates/linux_ci_workflow.yml.in

Can you see if you can simplify this docker logic here?

It'd be a good question to ask what it actually is trying to do and recreate the most minimally viable script from that.

For example, are the docker cp commands actually necessary here? Is the export id=$(docker run....) actually necessary or can we do it in a more simplistic docker run command?

yes, did that, the only reason docker cp is necessary in circleci is that their docker engine is wired remotely - https://circleci.com/docs/2.0/building-docker-images/#mounting-folders. While github action docker is running locally and volume mount is doable.

zhouzhuojie · 2021-05-06T18:04:52Z

.github/workflows/cancel_redundant_workflows.yml

generated from .github/scripts/regenerate_cancel_redundant_workflow.py

samestep · 2021-05-06T23:12:11Z

now that #57779 is merged, hopefully it should automatically run GHA workflows for you without someone else having to approve them... if not then I'll be sad haha

seemethere · 2021-05-10T17:07:16Z

.github/templates/linux_ci_workflow.yml.in

I think we probably don't need to do this here, the reason we were doing the whole docker commit && docker push was to preserve artifacts from one stage to another

This is for feature parity from circleci jobs. I think it can be useful if people rely on this debug image for quick iteration on their docs. Should we still remove it?

just checked that the ecr user in gha doesn't have push permission, will remove this then

facebook-github-bot · 2021-05-13T17:09:46Z

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

facebook-github-bot · 2021-05-13T22:44:30Z

@zhouzhuojie merged this pull request in 7756cb6.

Summary: # Changes This PR migrates `pytorch_python_doc_build` from circleci to github actions. Noticeable changes - Refactor `docker cp` into a single `docker run` with volume mount, because the in circleci volume is not accessible from its remote docker engine - `pytorch_python_doc_push` job will have a race condition with circleci, which will be migrated in separate PRs Pull Request resolved: pytorch#57371 Reviewed By: samestep Differential Revision: D28416289 Pulled By: zhouzhuojie fbshipit-source-id: 04caccccf3d7eb7e2225846a406a53ccda356d44

facebook-github-bot added the cla signed label Apr 30, 2021

zhouzhuojie force-pushed the zz/migrate-py-doc-build-gha branch 2 times, most recently from bf4a5f6 to 7d33d8d Compare May 3, 2021 02:59

zhouzhuojie changed the title ~~Migrate pytorch_python_doc_build to github action~~ DO NOT MERGE - Migrate pytorch_python_doc_build to github action May 3, 2021

zhouzhuojie marked this pull request as ready for review May 3, 2021 16:39

seemethere reviewed May 3, 2021

View reviewed changes

.github/workflows/doc_build.yml Outdated Show resolved Hide resolved

zhouzhuojie commented May 3, 2021

View reviewed changes

.github/workflows/doc_build.yml Outdated Show resolved Hide resolved

zhouzhuojie force-pushed the zz/migrate-py-doc-build-gha branch from 7d33d8d to be3ffae Compare May 3, 2021 18:11

zhouzhuojie commented May 3, 2021

View reviewed changes

zhouzhuojie force-pushed the zz/migrate-py-doc-build-gha branch 2 times, most recently from 10f1da9 to 36caf82 Compare May 3, 2021 18:42

zhouzhuojie commented May 3, 2021

View reviewed changes

.github/templates/linux_ci_workflow.yml.in Outdated

Copy link

Contributor Author

zhouzhuojie May 3, 2021

Choose a reason for hiding this comment

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

we can also move cpp docs job to here in the future

seemethere reviewed May 4, 2021

View reviewed changes

.github/templates/linux_ci_workflow.yml.in Outdated Show resolved Hide resolved

zhouzhuojie force-pushed the zz/migrate-py-doc-build-gha branch from 36caf82 to a414c14 Compare May 4, 2021 18:04

zhouzhuojie commented May 4, 2021

View reviewed changes

zhouzhuojie force-pushed the zz/migrate-py-doc-build-gha branch 3 times, most recently from 9be290e to 7a2ba0c Compare May 5, 2021 00:55

zhouzhuojie commented May 5, 2021

View reviewed changes

zhouzhuojie force-pushed the zz/migrate-py-doc-build-gha branch from 7a2ba0c to 2fed6f4 Compare May 5, 2021 15:53

seemethere reviewed May 5, 2021

View reviewed changes

samestep mentioned this pull request May 5, 2021

Migrate build/test jobs from CircleCI to GitHub Actions #57686

Closed

71 tasks

zhouzhuojie force-pushed the zz/migrate-py-doc-build-gha branch from 2fed6f4 to b67ee83 Compare May 6, 2021 18:04

zhouzhuojie commented May 6, 2021

View reviewed changes

zhouzhuojie force-pushed the zz/migrate-py-doc-build-gha branch 2 times, most recently from fb801b7 to df3cfd3 Compare May 6, 2021 22:26

zhouzhuojie force-pushed the zz/migrate-py-doc-build-gha branch 4 times, most recently from d702293 to 82af939 Compare May 10, 2021 17:03

zhouzhuojie changed the title ~~DO NOT MERGE - Migrate pytorch_python_doc_build to github action~~ Migrate pytorch_python_doc_build to github action May 10, 2021

seemethere reviewed May 10, 2021

View reviewed changes

zhouzhuojie force-pushed the zz/migrate-py-doc-build-gha branch 2 times, most recently from 718d399 to fcccfef Compare May 12, 2021 17:30

Migrate pytorch_python_doc_build to github action

4c5fe73

zhouzhuojie force-pushed the zz/migrate-py-doc-build-gha branch from fcccfef to 4c5fe73 Compare May 13, 2021 16:45

samestep approved these changes May 13, 2021

View reviewed changes

facebook-github-bot closed this in 7756cb6 May 13, 2021

facebook-github-bot added the Merged label May 13, 2021

zhouzhuojie deleted the zz/migrate-py-doc-build-gha branch May 13, 2021 22:46

Migrate pytorch_python_doc_build to github action #57371

Migrate pytorch_python_doc_build to github action #57371

Uh oh!

Conversation

zhouzhuojie commented Apr 30, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Uh oh!

facebook-github-bot commented Apr 30, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

💊 CI failures summary and remediations

Uh oh!

codecov bot commented May 3, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zhouzhuojie commented May 3, 2021

Uh oh!

zhouzhuojie commented May 3, 2021

Uh oh!

zhouzhuojie commented May 3, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

samestep commented May 4, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

samestep commented May 4, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

samestep commented May 4, 2021

Uh oh!

zhouzhuojie commented May 4, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

samestep commented May 5, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

samestep commented May 6, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot commented May 13, 2021

Uh oh!

facebook-github-bot commented May 13, 2021

Uh oh!

Reviewers

Assignees

zhouzhuojie commented Apr 30, 2021 •

edited

Loading

facebook-github-bot commented Apr 30, 2021 •

edited

Loading

codecov bot commented May 3, 2021 •

edited

Loading

samestep commented May 4, 2021 •

edited

Loading

samestep commented May 4, 2021 •

edited

Loading

samestep commented May 6, 2021 •

edited

Loading