Skip to content

Conversation

@huydhn
Copy link
Contributor

@huydhn huydhn commented Aug 24, 2022

Cache all python dependencies using GHA cache. I'm doing this for lint workflow first and will slowly roll it out to other workflows.

Testing

Before caching, pip cache is not found. Dependencies installation continues as usual:

Screen Shot 2022-08-24 at 16 36 15

After caching https://github.com/pytorch/pytorch/runs/8006214772?check_suite_focus=true. The long hash at the end of the cache key is the hash of requirements files

Screen Shot 2022-08-24 at 16 51 51

Note that the cache is in the runners themselves. This should be a transparent process.

@pytorch-bot
Copy link

pytorch-bot bot commented Aug 24, 2022

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/84026

Note: Links to docs will display an error until the docs builds have been completed.

✅ No Failures

As of commit 645b2ec:
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@huydhn huydhn changed the title [BE] Enable cache action for lint workflow Enable cache action for lint workflow Aug 24, 2022
@huydhn huydhn self-assigned this Aug 24, 2022
@huydhn huydhn requested a review from seemethere August 25, 2022 00:00
@huydhn huydhn marked this pull request as ready for review August 25, 2022 00:00
@huydhn huydhn requested a review from a team as a code owner August 25, 2022 00:00
@huydhn huydhn removed the request for review from seemethere August 25, 2022 00:33
@huydhn huydhn marked this pull request as draft August 25, 2022 00:33
@huydhn huydhn marked this pull request as ready for review August 25, 2022 01:54
@huydhn huydhn requested a review from seemethere August 25, 2022 01:56
Copy link
Member

@suo suo left a comment

Choose a reason for hiding this comment

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

what's the expected impact on this change on TTS?

@huydhn
Copy link
Contributor Author

huydhn commented Aug 25, 2022

what's the expected impact on this change on TTS?

Not much I think because installing dependencies isn't the long pole in term of TTS. However, it has a minor reliability boon that allows the cached copies to be used whenever there are upstream issues with pip. And the setup is pretty easy as you can see in the PR. Basically, we just need to add to setup-python

cache: pip
cache-dependency-path: requirements

Copy link
Member

@suo suo left a comment

Choose a reason for hiding this comment

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

coolio

@malfet
Copy link
Contributor

malfet commented Aug 25, 2022

Looks good, but please avoid images in PR description, rather share link to respective runs + text-readable perf numbers

@huydhn
Copy link
Contributor Author

huydhn commented Aug 25, 2022

@pytorchbot merge

@pytorchmergebot
Copy link
Collaborator

@pytorchbot successfully started a merge job. Check the current status here and land check progress here.
The merge job was triggered with the land checks (-l) flag. If you did not specify this flag yourself, you are likely enrolled in the land checks rollout. This means that your change will be merged once all checks on your PR and the land checks have passed (ETA 4 Hours). If you need to coordinate lands between different changes and cannot risk a land race, please add the ciflow/trunk label to your PR and wait for signal to complete, and then land your changes in proper order. Having trunk, pull, and Lint pre-run on a PR will bypass land checks and the ETA should be immediate. If this is not the intended behavior, feel free to use some of the other merge options in the wiki.
Please reach out to the PyTorch DevX Team with feedback or questions!

pytorchmergebot pushed a commit that referenced this pull request Aug 25, 2022
Cache all python dependencies using [GHA cache](https://docs.github.com/en/actions/using-workflows/caching-dependencies-to-speed-up-workflows).  I'm doing this for lint workflow first and will slowly roll it out to other workflows.

### Testing

Before caching, pip cache is not found. Dependencies installation continues as usual:

![Screen Shot 2022-08-24 at 16 36 15](https://user-images.githubusercontent.com/475357/186543554-9d7f5978-2c2d-4362-9535-c3b17e922da1.png)

After caching https://github.com/pytorch/pytorch/runs/8006214772?check_suite_focus=true. The long hash at the end of the cache key is the hash of requirements files

![Screen Shot 2022-08-24 at 16 51 51](https://user-images.githubusercontent.com/475357/186543825-055ea025-3d42-42fc-877d-baec358de0ed.png)

Note that the cache is in the runners themselves. This should be a transparent process.

Pull Request resolved: #84026
Approved by: https://github.com/seemethere, https://github.com/suo, https://github.com/malfet
@github-actions
Copy link
Contributor

Hey @huydhn.
You've committed this PR, but it does not have both a 'release notes: ...' and 'topics: ...' label. Please add one of each to the PR. The 'release notes: ...' label should represent the part of PyTorch that this PR changes (fx, autograd, distributed, etc) and the 'topics: ...' label should represent the kind of PR it is (not user facing, new feature, bug fix, perf improvement, etc). The list of valid labels can be found here for the 'release notes: ...' and here for the 'topics: ...'.
For changes that are 'topic: not user facing' there is no need for a release notes label.

@malfet
Copy link
Contributor

malfet commented Aug 26, 2022

One needs to be careful with introducing new dependencies, as lint will fail for all users that have not rebased, as workflow definition is checked from master, but rest of the code is from PR, which results in the following failures:
https://github.com/pytorch/pytorch/runs/8020179452?check_suite_focus=true

@huydhn
Copy link
Contributor Author

huydhn commented Aug 26, 2022

One needs to be careful with introducing new dependencies, as lint will fail for all users that have not rebased, as workflow definition is checked from master, but rest of the code is from PR, which results in the following failures: https://github.com/pytorch/pytorch/runs/8020179452?check_suite_focus=true

Note. In hindsight, I should have added the new file first, then made the change.

pytorchmergebot pushed a commit that referenced this pull request Aug 26, 2022
Following up on #84026, these are the rest of pip dependencies that I can find.

Pull Request resolved: #84093
Approved by: https://github.com/malfet
facebook-github-bot pushed a commit that referenced this pull request Aug 26, 2022
Summary:
Cache all python dependencies using [GHA cache](https://docs.github.com/en/actions/using-workflows/caching-dependencies-to-speed-up-workflows).  I'm doing this for lint workflow first and will slowly roll it out to other workflows.

### Testing

Before caching, pip cache is not found. Dependencies installation continues as usual:

![Screen Shot 2022-08-24 at 16 36 15](https://user-images.githubusercontent.com/475357/186543554-9d7f5978-2c2d-4362-9535-c3b17e922da1.png)

After caching https://github.com/pytorch/pytorch/runs/8006214772?check_suite_focus=true. The long hash at the end of the cache key is the hash of requirements files

![Screen Shot 2022-08-24 at 16 51 51](https://user-images.githubusercontent.com/475357/186543825-055ea025-3d42-42fc-877d-baec358de0ed.png)

Note that the cache is in the runners themselves. This should be a transparent process.

Pull Request resolved: #84026
Approved by: https://github.com/seemethere, https://github.com/suo, https://github.com/malfet

Test Plan: contbuild & OSS CI, see https://hud.pytorch.org/commit/pytorch/pytorch/02c3781332031981988cd0cadfd573a210210b33

Reviewed By: weiwangmeta

Differential Revision: D39025475

Pulled By: huydhn

fbshipit-source-id: e4506911cb662bcf4c32db4225acdb0acd9da760
pytorchmergebot pushed a commit that referenced this pull request Aug 26, 2022
Following up on #84026, these are the rest of pip dependencies that I can find.

Pull Request resolved: #84093
Approved by: https://github.com/malfet
facebook-github-bot pushed a commit that referenced this pull request Aug 28, 2022
…84093)

Summary:
Following up on #84026, these are the rest of pip dependencies that I can find.

Pull Request resolved: #84093
Approved by: https://github.com/malfet

Test Plan: contbuild & OSS CI, see https://hud.pytorch.org/commit/pytorch/pytorch/82efb0e196c71a75985595fbbf294d8c816e9753

Reviewed By: weiwangmeta

Differential Revision: D39084828

Pulled By: huydhn

fbshipit-source-id: 2fddfcddd7291f1ee57bc697fd9b18ab2c67a1f0
facebook-github-bot pushed a commit that referenced this pull request Aug 29, 2022
…84093)

Summary:
Following up on #84026, these are the rest of pip dependencies that I can find.

Pull Request resolved: #84093
Approved by: https://github.com/malfet

Test Plan: contbuild & OSS CI, see https://hud.pytorch.org/commit/pytorch/pytorch/82efb0e196c71a75985595fbbf294d8c816e9753

Reviewed By: weiwangmeta

Differential Revision: D39089768

Pulled By: huydhn

fbshipit-source-id: f5ae04ce6ca885a0255ead44dd583b160ebe35d5
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.

6 participants