Skip to content

Conversation

@pstibrany
Copy link
Contributor

@pstibrany pstibrany commented Sep 24, 2020

What this PR does: This PR refactors ObjectClient interface to explicitly pass delimiter to List method. Previously delimiter was property of ObjectClient passed to it when created, but that doesn't work if code using ObjectClient wants to do both recursive and non-recursive calls.

This PR also changes FSObjectClient such that it always expects paths using / separator, but maps it to \ on Windows internally. Code using FSObjectClient will only see / returned.

This is a pre-requisite for PR #3235.

Checklist

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

@pstibrany
Copy link
Contributor Author

/cc @sandeepsukhani since Loki uses this.

Signed-off-by: Peter Štibraný <[email protected]>
Copy link
Contributor

@sandeepsukhani sandeepsukhani left a comment

Choose a reason for hiding this comment

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

A small suggested change in FS object client otherwise it LGTM.

Copy link
Contributor

@pracucci pracucci left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Contributor

@pracucci pracucci left a comment

Choose a reason for hiding this comment

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

Few final nits!

Signed-off-by: Peter Štibraný <[email protected]>
@pstibrany
Copy link
Contributor Author

Addressed remaining comments. I've also copied List description from #3235 into this PR.

Signed-off-by: Peter Štibraný <[email protected]>
Signed-off-by: Peter Štibraný <[email protected]>
@pstibrany pstibrany merged commit 35a621c into cortexproject:master Sep 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants