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

feat: pass region to minio storage backend #478

Merged
merged 6 commits into from
Apr 22, 2024

Conversation

imnotjames
Copy link
Contributor

@imnotjames imnotjames commented Apr 2, 2024

Purpose/Motivation

without the region being set in the minio client, using codecov with garage, the s3 compatible storage system fails

Links to relevant tickets

dependent on codecov/shared#167 being merged in Good to go!

What does this PR do?

allow a user to set the minio client region when creating a client

bumps the shared version from 203b67e8b0fb066f7dde8dcfb53896c20c28b06e to a5b50e79697350abe9305a36bb10f23ef71827ae

See commits that this will bring in here

requirements.txt & requirements.in already have the commit needed from shared (a5b50e79697350abe9305a36bb10f23ef71827ae)

Notes to Reviewer

tested with garage 0.9.3 and minio (whatever the latest tag points at)

Legal Boilerplate

Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. In 2022 this entity acquired Codecov and as result Sentry is going to need some rights from me in order to utilize my contributions in this PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.

@imnotjames imnotjames marked this pull request as ready for review April 2, 2024 06:51
@imnotjames imnotjames marked this pull request as draft April 3, 2024 22:59
@imnotjames
Copy link
Contributor Author

imnotjames commented Apr 3, 2024

Converting to draft until the dependent PR is merged All set.

@imnotjames imnotjames marked this pull request as ready for review April 4, 2024 16:54
@thomasrockhu-codecov
Copy link
Contributor

related to codecov/shared#167

@imnotjames
Copy link
Contributor Author

I've merged main into this branch. The main revision of requirements.txt & requirements.in already have the dependent commit included, so this now only changes the code needed to pass region.

@imnotjames
Copy link
Contributor Author

Anything I can do to help this get reviewed? I'd be thrilled to not have to patch the docker container to support this feature.

@trent-codecov trent-codecov added this pull request to the merge queue Apr 22, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Apr 22, 2024
@trent-codecov trent-codecov added this pull request to the merge queue Apr 22, 2024
Merged via the queue into codecov:main with commit 9eebb75 Apr 22, 2024
11 checks passed
@imnotjames
Copy link
Contributor Author

Oh, cool, merge queue! Y'all rock, thanks.

@trent-codecov
Copy link
Contributor

You are the actually the first PR we have done this with. Congrats!! and thanks for the change!

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.

None yet

3 participants