[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.

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

Fix object prefix not being stripped for Dell ECS #561

Merged
merged 3 commits into from
Mar 1, 2021

Conversation

kradalby
Copy link
Contributor

What this PR does:
This commit ensure that the same object prefix string is used to query and strip
an object when listing all the objects in a bucket.

Some S3 implementations (in this case Dell ECS) cannot be relied upon to return
the correct res.Prefix. Dell ECS will return it URLencoded, e.g. "traceID%2F"
instead of "traceID/".

I am able to test this on ECS, but I do not have access to other blob storage, so help with testing additional storage backend would be appreciated.

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

Checklist

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

This commit ensure that the same object prefix string is used to query and strip
an object when listing all the objects in a bucket.

Some S3 implementations (in this case Dell ECS) cannot be relied upon to return
the correct `res.Prefix`. Dell ECS will return it URLencoded, e.g. "traceID%2F"
instead of "traceID/".
@CLAassistant
Copy link
CLAassistant commented Feb 26, 2021

CLA assistant check
All committers have signed the CLA.

@mdisibio
Copy link
Contributor

Hi thanks for taking care of this! It's looking good for S3 and MinIO. If you can confirm it fixes Dell ECS then we'll merge.

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 @kradalby!

@kradalby
Copy link
Contributor Author
kradalby commented Mar 1, 2021

Awesome, thanks, I am getting it built and tested today, got distracted on Friday.

CHANGELOG.md Outdated Show resolved Hide resolved
Co-authored-by: Graham Reed <greed@7deadly.org>
@kradalby kradalby marked this pull request as ready for review March 1, 2021 13:38
@kradalby
Copy link
Contributor Author
kradalby commented Mar 1, 2021

I can confirm that it works with Dell ECS, I have played around with it today, read, find, compact, write cycles seem to work as intended, there are no longer any errors thrown in the log.

@mdisibio mdisibio merged commit 60e2722 into grafana:master Mar 1, 2021
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.

error polling blocklist, fails to list blocks on Dell ECS
5 participants