Explicitly fetch PR head in metrics diff job #3627
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
alecgrieser
commented
Sep 24, 2025
| metrics-analysis: | ||
| runs-on: ubuntu-latest | ||
| permissions: | ||
| contents: read |
There was a problem hiding this comment.
We should have added this to say that this job is allowed to read contents from the git repo. The fact that we didn't wasn't causing us trouble (perhaps because the repo is public), but this is good practice.
normen662
approved these changes
Sep 24, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The metrics diff job failed when trying to compare two hashes: https://github.com/FoundationDB/fdb-record-layer/actions/runs/17979075422
This is because we are using the
pull_request_targetjob type, and that only fetches the target's branch's state (so,mainin this case), but we need to fetch both the PR head and the target head in order to compute the diff. We have to be a little careful, because thepull_request_targetruns at heightened permissions (that is, including writes), so it is important it does not run any outside code. So in this fix, we fetch the PR head, but we do not check it out. We will continue to run the analysis tool from the target branch.Hopefully, this unblocks the broken metrics diff job.