Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: ensure pull-requests properly use cached tasks #389

Merged
merged 1 commit into from
Dec 15, 2023
Merged

fix: ensure pull-requests properly use cached tasks #389

merged 1 commit into from
Dec 15, 2023

Conversation

ahal
Copy link
Collaborator

@ahal ahal commented Dec 13, 2023

Fixes issues #222 and #359

@ahal ahal self-assigned this Dec 13, 2023
@ahal ahal marked this pull request as draft December 13, 2023 20:54
bhearsum
bhearsum reviewed Dec 13, 2023
View reviewed changes
Copy link
Contributor

@bhearsum bhearsum left a comment

Choose a reason for hiding this comment

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

This needs tests and ideally some real world testing but it looks very sane to me. If you'd like, I can give it a try in Translations before it goes up for proper review and/or lands (I doubt it would cause any serious bustage if it causes an issues at all...)

src/taskgraph/util/cached_tasks.py Show resolved Hide resolved
src/taskgraph/util/cached_tasks.py Outdated Show resolved Hide resolved
@bhearsum
Copy link
Contributor

I gave this a quick try in a PR in translations: mozilla/translations#303

It looks good there. The first push busted all the caches due to the taskgraph update, but the tasks appear to have the correct routes. An additional push optimized away all of the fetch and toolchain tasks, as expected.

This doesn't verify anything about fallback back to level 1 caches, but it's a good sign for this patch!

@ahal
Copy link
Collaborator Author

ahal commented Dec 14, 2023

Thanks for testing! And yes, I still plan on adding tests to this

@ahal ahal mentioned this pull request Dec 14, 2023
Merged
@ahal ahal marked this pull request as ready for review December 15, 2023 14:47
@ahal
Copy link
Collaborator Author

ahal commented Dec 15, 2023

The tests were added in #394, so this PR just updates the expectations.

@ahal ahal requested a review from bhearsum December 15, 2023 15:41
@ahal ahal merged commit 26efabe into taskcluster:main Dec 15, 2023
7 checks passed
@ahal ahal deleted the pr_cached_tasks branch December 15, 2023 22:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bhearsum bhearsum bhearsum approved these changes

Assignees

@ahal ahal

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ahal @bhearsum