-
Notifications
You must be signed in to change notification settings - Fork 251
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
Check an object store is on local file system #918
Conversation
rust/src/io/object_store.rs
Outdated
@@ -41,7 +41,7 @@ use super::object_reader::ObjectReader; | |||
pub struct ObjectStore { | |||
// Inner object store | |||
pub inner: Arc<dyn OSObjectStore>, | |||
scheme: String, | |||
pub scheme: String, |
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.
Does this need to be public if we have is_local
?
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.
ok, i can make it private for now
@@ -147,6 +147,11 @@ impl ObjectStore { | |||
} | |||
} | |||
|
|||
/// Returns true if the object store pointed to a local file system. | |||
pub fn is_local(&self) -> bool { | |||
self.scheme == "file" |
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.
new_from_path
creates a store with scheme flle
- I never figured out why, but can this be a problem here?
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.
new_from_path creates a local object store, right? that's the correct expectation?
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 mean there is a type there - flle
instead of file
, but I don't know why:
lance/rust/src/io/object_store.rs
Line 115 in e4aa45c
scheme: String::from("flle"), |
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, this appears to be a typo. lemme fix in this PR then
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
No description provided.