Skip to content

Conversation

@mattip
Copy link
Contributor

@mattip mattip commented Jun 7, 2021

fixes issue #3997

The important part is to add a filter for tags, otherwise the tag commit does not trigger a documentation build and upload.

Copy link
Member

@NicolasHug NicolasHug left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @mattip!

I'm trying to understand a few things:

The important part is to add a filter for tags, otherwise the tag commit does not trigger a documentation build and upload.

Why do we only need the tag for binary_linux_wheel_py3.7_cpu?

The CircleCI docs say:

CircleCI does not run workflows for tags unless you explicitly specify tag filters. Additionally, if a job requires any other jobs (directly or indirectly), you must specify tag filters for those jobs.

So, since the dependency between our jobs is upload_docs -> build_docs -> binary_linux_wheel_py3.7_cpu, shouldn't we also add the tag filter for build_docs, which is currently missing?

fb = "master"
if not fb and (os_type == 'linux' and cu_version == 'cpu' and
btype == 'wheel' and python_version =='3.7'):
# this is for build_docs, the versions must match
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure I understand this comment: what is "versions", and what should it match with?

@mattip
Copy link
Contributor Author

mattip commented Jun 8, 2021

shouldn't we also add the tag filter for build_docs, which is currently missing?

Good point. Added and rebased off master.

Copy link
Contributor

@datumbox datumbox left a comment

Choose a reason for hiding this comment

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

@mattip Thanks for fixing it. There are some related lint issues. Could you please check?

Copy link
Contributor

@datumbox datumbox left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@seemethere
Copy link
Member

So the way we actually do this in upstream pytorch/pytorch is by specifying a /.*/ branch in the filter branch to run on all branches:

https://github.com/pytorch/pytorch/blob/b844fd11eeacf0391fd904e5508bda1af54664f8/.circleci/config.yml#L6780-L6787

@mattip
Copy link
Contributor Author

mattip commented Jun 11, 2021

Anything else needed here? It seems this goes out-of-date every night, so if needed I can update

@malfet malfet merged commit 26d6080 into pytorch:master Jun 11, 2021
@github-actions
Copy link

Hey @datumbox, @malfet!

You approved or merged this PR, but no labels were added.

if not filter_branch:
# Build on every pull request, but upload only on nightly and tags
w += build_doc_job(None)
w += build_doc_job('nightly')
Copy link
Member

Choose a reason for hiding this comment

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

@mattip does this mean that the docs are not build on every PR anymore?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hmm. It seems to be building on PRs, for instance taking a random PR and looking at the CI build I see a build_docs run 2/3 of the way down. Although I agree the filter says only nightly, so it is misleading.

      - build_docs:
          filters:
            branches:
              only:
              - nightly
            tags:
              only: /v[0-9]+(\.[0-9]+)*-rc[0-9]+/
          name: build_docs
          python_version: '3.7'
          requires:
          - binary_linux_wheel_py3.7_cpu

Copy link
Member

Choose a reason for hiding this comment

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

I'm afraid the PR you're linking to was pushed before this one got merged.

In more recent commits, e.g. https://app.circleci.com/pipelines/github/pytorch/vision?branch=pull%2F3808 from this morning, I do not see the build_docs job.

facebook-github-bot pushed a commit that referenced this pull request Jun 14, 2021
Summary:
* add filter for binary_linux_wheel_py3.7_cpu
fixes issue #3997

* add tag,nightly filter to build_docs

* flake8 formatting

* fixes from review

Reviewed By: fmassa

Differential Revision: D29097738

fbshipit-source-id: d4c6a0b3b9d3bfc9a20fffd003748cd417b3a877
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants