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

Using directory as prefix for S3 #1720

Merged
merged 3 commits into from
Aug 19, 2021
Merged

Using directory as prefix for S3 #1720

merged 3 commits into from
Aug 19, 2021

Conversation

linzhp
Copy link
Contributor

@linzhp linzhp commented Jun 23, 2021

What is the problem I am trying to address?

Listing objects on S3 with prefixes like github.com/golang/mock/@v/v1.4.4. with return empty list, because prefixes are directories on S3 (https://docs.aws.amazon.com/AmazonS3/latest/userguide/using-prefixes.html).

How is the fix applied?

Taking the directory name from the path returned by config.PackageVersionedName(module, version, ""), so it can return all files for the module. Because of this, all files from all versions will be returned and filtered later.

@linzhp linzhp requested a review from a team as a code owner June 23, 2021 05:33
Copy link
Contributor

@marwan-at-work marwan-at-work left a comment

Choose a reason for hiding this comment

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

Thanks for the fix. I'm surprised this hasn't come up before.

I think we could really use some integration testing here but unfortunately I don't have an AWS account.

cc: @arschles

pkg/storage/s3/checker.go Outdated Show resolved Hide resolved
@linzhp
Copy link
Contributor Author

linzhp commented Jul 9, 2021

@marwan-at-work Can you take another look?

@marwan-at-work
Copy link
Contributor

@linzhp just a nit comment and looks good 👍🏼

@marwan-at-work marwan-at-work merged commit 4abe908 into gomods:main Aug 19, 2021
@linzhp linzhp deleted the dir branch August 19, 2021 01:15
linzhp added a commit to linzhp/athens that referenced this pull request Aug 26, 2022
Summary:
* change configuration to use S3 as the storage client
* configure parameters to call  Terrablob with S3 API according to https://engwiki.uberinternal.com/display/TERRABLOB/S3+HTTP+API
* Patch the S3 client to put all modules under a path prefix that we use in Terrablob
* Patch a pending pull request gomods#1720

Test Plan: * deployed to staging, change GOPROXY to point to staging, and run `go clean -modcache && go mod tidy`

Reviewers: tanx, sergeyb

Reviewed By: tanx, sergeyb

JIRA Issues: GM-673

Differential Revision: https://code.uberinternal.com/D6235753
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants