-
Notifications
You must be signed in to change notification settings - Fork 820
Refactoring: use PrefixedBucketClient in UserBucketClient #3870
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
Refactoring: use PrefixedBucketClient in UserBucketClient #3870
Conversation
Signed-off-by: Marco Pracucci <marco@pracucci.com>
@@ -23,7 +22,7 @@ func NewPrefixedBucketClient(bucket objstore.Bucket, prefix string) *PrefixedBuc | |||
} | |||
|
|||
func (b *PrefixedBucketClient) fullName(name string) string { | |||
return path.Join(b.prefix, name) | |||
return b.prefix + objstore.DirDelim + name |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To keep the same exact logic we had in UserBucketClient
. If name is "", the delimiter must be added anyway.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, sorry I made this change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on my suggestion, so sorry about that :)
I'm not sure empty name should be a valid choice though and I wonder how do object stores work in that case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure empty name should be a valid choice
It's used to list the root. Thanos does it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah right, I misunderstood and was thinking about empty prefix.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
What this PR does:
This is just an internal refactoring PR to use
PrefixedBucketClient
inUserBucketClient
.Which issue(s) this PR fixes:
N/A
Checklist
CHANGELOG.md
updated - the order of entries should be[CHANGE]
,[FEATURE]
,[ENHANCEMENT]
,[BUGFIX]