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

sha256 for binaries index #6241

Merged
merged 1 commit into from
Feb 3, 2025
Merged

sha256 for binaries index #6241

merged 1 commit into from
Feb 3, 2025

Conversation

clee2000
Copy link
Contributor

@clee2000 clee2000 commented Jan 31, 2025

Restores the sha256 hash for binaries if they exist.

In pytorch/pytorch#144887 the sha256 of the binary is manually calculated and uploaded as metadata with the binary because the automatic one given by s3 is wrong (likely due to it being part of a multipart upload https://docs.aws.amazon.com/AmazonS3/latest/userguide/tutorial-s3-mpu-additional-checksums.html).

This PR

  • Attempts to adds a check for multipart upload sha256's and does not show them if they are part of a multipart upload and
  • Surfaces the new metadata given by the above PR
  • Restores the sha256 that was previously removed due to being wrong

I also went back and corrected the bad shas from the multi part upload (not that it would have mattered, as this PR wouldn't show them in the index). The semantics of copy_from which is used in managed.py if --compute-sha256 is used is correct and doesn't have the multi part upload issue

Copy link

vercel bot commented Jan 31, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Skipped Deployment
Name Status Preview Updated (UTC)
torchci ⬜️ Ignored (Inspect) Visit Preview Feb 3, 2025 6:20pm

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jan 31, 2025
@clee2000 clee2000 force-pushed the csl/test_manage_collect_bad branch from 37d28b1 to ab2a21f Compare February 3, 2025 18:20
@clee2000 clee2000 changed the title [test] s3 gather bad checksums sha256 for binaries index Feb 3, 2025
@clee2000 clee2000 marked this pull request as ready for review February 3, 2025 18:27
@clee2000 clee2000 requested a review from atalman February 3, 2025 18:27
Copy link
Contributor

@atalman atalman left a comment

Choose a reason for hiding this comment

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

lgtm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants