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.

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

Set oldest_content_accepted for remote downloader requests without the checksum #23970

Closed
wants to merge 3 commits into from

Conversation

mislavmandaricaxilis
Copy link
Contributor

@mislavmandaricaxilis mislavmandaricaxilis commented Oct 14, 2024

This PR address the #23932 issue with remote downloader.

@@ -90,6 +93,8 @@ public class GrpcRemoteDownloader implements AutoCloseable, Downloader {
// value when both are present.
private static final String QUALIFIER_HTTP_HEADER_URL_PREFIX = "http_header_url:";

private Clock clock = Clock.systemUTC();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Instead of storing a clock here, you can use com.google.devtools.build.lib.clock.BlazeClock#instance, which is still injectable for testing purposes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the tip. I replaced the usage of java.time.Clock with com.google.devtools.build.lib.clock.BlazeClock and made appropriate changes to remove the manual property injection into the GrpcRemoteDownloader class.

Copy link
Collaborator

@fmeum fmeum left a comment

Choose a reason for hiding this comment

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

Co-authored-by: Fabian Meumertzheim <fabian@meumertzhe.im>
@mislavmandaricaxilis mislavmandaricaxilis marked this pull request as ready for review October 14, 2024 15:01
@mislavmandaricaxilis mislavmandaricaxilis requested a review from a team as a code owner October 14, 2024 15:02
@github-actions github-actions bot added team-Remote-Exec Issues and PRs for the Execution (Remote) team awaiting-review PR is awaiting review from an assigned reviewer labels Oct 14, 2024
@meteorcloudy
Copy link
Member

@coeuvre Please take a look

@coeuvre coeuvre added awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally and removed awaiting-review PR is awaiting review from an assigned reviewer labels Oct 14, 2024
@fmeum
Copy link
Collaborator

fmeum commented Oct 14, 2024

@bazel-io fork 8.0.0

@github-actions github-actions bot removed the awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally label Oct 15, 2024
bazel-io pushed a commit to bazel-io/bazel that referenced this pull request Oct 15, 2024
…e checksum

This PR address the bazelbuild#23932 issue with remote downloader.

Closes bazelbuild#23970.

PiperOrigin-RevId: 686180819
Change-Id: Ia36c5793622bd1ac1fdaa9ef1326ddc385a5a01f
github-merge-queue bot pushed a commit that referenced this pull request Oct 22, 2024
…thout the checksum (#23995)

This PR address the #23932 issue with remote downloader.

Closes #23970.

PiperOrigin-RevId: 686180819
Change-Id: Ia36c5793622bd1ac1fdaa9ef1326ddc385a5a01f

Commit
60638af

Co-authored-by: Mislav Mandaric <mislav.mandaric@happening.xyz>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team-Remote-Exec Issues and PRs for the Execution (Remote) team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants