-
Notifications
You must be signed in to change notification settings - Fork 867
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
feat(object_store): add PrefixObjectStore #3329
Conversation
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.
Just some minor nits
object_store/src/prefix.rs
Outdated
let prefix = prefix.and_then(|p| self.full_path(p).ok()); | ||
Ok(self | ||
.inner | ||
.list(Some(&prefix.unwrap_or_else(|| self.prefix.clone()))) |
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 think this could be simpler with just a basic match
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.
Not entirely sure, if my update here is what you had in mind :)
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.
Oh I see what complicates this - suggestion in https://github.com/apache/arrow-rs/pull/3329/files#r1045836725
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.
much cleaner with infallible full_path
- hope that's what you had in mind.
Co-authored-by: Raphael Taylor-Davies <1781103+tustvold@users.noreply.github.com>
Benchmark runs are scheduled for baseline = ad94368 and contender = 225ea93. 225ea93 is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
Which issue does this PR close?
Closes #3328
Rationale for this change
see #3328
What changes are included in this PR?
Adds a new
PrefixObjectStore
Are there any user-facing changes?
yes, a new object store wrapper is available :)