[go: nahoru, domu]
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.

Sign up for GitHub

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

Jump to bottom

Remove s3 bucket creation #404

Merged
merged 5 commits into from
Dec 11, 2020
Merged

Remove s3 bucket creation #404

merged 5 commits into from
Dec 11, 2020

Conversation

dgzlopes
Copy link
Member

What this PR does:
Removes the code that creates the S3 bucket.

Which issue(s) this PR fixes:
Fixes #401

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

@dgzlopes
Remove s3 bucket creation
f1e41ef 
Signed-off-by: Daniel González Lopes <danielgonzalezlopes@gmail.com>
@dgzlopes
Remove s3 bucket creation
6507bd4 
Signed-off-by: Daniel González Lopes <danielgonzalezlopes@gmail.com>
@mdisibio
Copy link
Contributor

This is great thank you.

A couple thoughts:

  • Do you know if the min.io example needs to be updated to create the bucket initially, or is ok?
  • I think we can remove the s3 config Region value entirely now. Min.IO can infer the region from the endpoint url.

@dgzlopes
Reemove s3 region config flag
7cee618 
Signed-off-by: Daniel González Lopes <danielgonzalezlopes@gmail.com>
@dgzlopes
Copy link
Member Author
dgzlopes commented Dec 10, 2020 •
edited
Loading

Thank you for taking the time to review :)

  • I'm not fully sure, I'll research it tomorrow - I've to leave.
  • Nice! Removed.

@dgzlopes dgzlopes changed the title Remove s3 bucket creation [WIP] Remove s3 bucket creation Dec 10, 2020
@dgzlopes
Rmv minio region
731d97e 
Signed-off-by: Daniel González Lopes <danielgonzalezlopes@gmail.com>
@dgzlopes
Copy link
Member Author
dgzlopes commented Dec 11, 2020 •
edited
Loading

@mdisibio In theory, the Minio example should work b/c here:

# from docker-compose.s3.minio.yaml line:24
mkdir -p /data/tempo && /usr/bin/minio server /data

When we create the new folder, we're creating a new bucket too.

@dgzlopes dgzlopes changed the title [WIP] Remove s3 bucket creation Remove s3 bucket creation Dec 11, 2020
@dgzlopes dgzlopes marked this pull request as ready for review December 11, 2020 07:48
@dgzlopes
Add to changelog
70be092 
Signed-off-by: Daniel González Lopes <danielgonzalezlopes@gmail.com>
annanay25
annanay25 approved these changes Dec 11, 2020
View reviewed changes
Copy link
Contributor
@annanay25 annanay25 left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks @dgzlopes!

@annanay25 annanay25 merged commit a35cd54 into grafana:master Dec 11, 2020
@dgzlopes dgzlopes deleted the remove-s3-bucket-creation branch December 11, 2020 10:03
@faceair
Copy link
faceair commented Dec 22, 2020

Hi guys, the region field is useful.

There are many s3-compatible object stores that rely on this field when calculating signatures, and their endpoint is completely different from s3. For example, Baidu's object store https://cloud.baidu.com/doc/BOS/s/xjwvyq9l4

@dgzlopes dgzlopes mentioned this pull request Jan 29, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@annanay25 annanay25 annanay25 approved these changes

@achatterjee-grafana achatterjee-grafana Awaiting requested review from achatterjee-grafana

@joe-elliott joe-elliott Awaiting requested review from joe-elliott

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Remove Creation of s3 Bucket from s3 Backend
4 participants
@dgzlopes @mdisibio @faceair @annanay25