Skip to content

Remove s3 bucket creation#404

Merged
annanay25 merged 5 commits intografana:masterfrom
dgzlopes:remove-s3-bucket-creation
Dec 11, 2020
Merged

Remove s3 bucket creation#404
annanay25 merged 5 commits intografana:masterfrom
dgzlopes:remove-s3-bucket-creation

Conversation

@dgzlopes
Copy link
Copy Markdown
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]

Signed-off-by: Daniel González Lopes <danielgonzalezlopes@gmail.com>
Signed-off-by: Daniel González Lopes <danielgonzalezlopes@gmail.com>
@mdisibio
Copy link
Copy Markdown
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.

Signed-off-by: Daniel González Lopes <danielgonzalezlopes@gmail.com>
@dgzlopes
Copy link
Copy Markdown
Member Author

dgzlopes commented Dec 10, 2020

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
Signed-off-by: Daniel González Lopes <danielgonzalezlopes@gmail.com>
@dgzlopes
Copy link
Copy Markdown
Member Author

dgzlopes commented Dec 11, 2020

@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
Signed-off-by: Daniel González Lopes <danielgonzalezlopes@gmail.com>
Copy link
Copy Markdown
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
Copy Markdown

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

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.

Remove Creation of s3 Bucket from s3 Backend

4 participants