tustvold commented on code in PR #4077:
URL: https://github.com/apache/arrow-rs/pull/4077#discussion_r1165406088
##########
object_store/src/lib.rs:
##########
@@ -717,6 +726,139 @@ impl From<Error> for std::io::Error {
}
}
+/// Creates object store from provided url and options
+///
+/// The scheme of the provided url is used to instantiate the store. If the url
+/// scheme is cannot be mapped to a store, [`NotImplemented`] is raised. For
invalid
+/// input, e.g. url with no scheme the default behaviour is to return
+/// [`local::LocalFileSystem`].
+///
+/// # Examples
+///
+/// ```
+///
+/// ```
+pub fn parse_url<
+ I: IntoIterator<Item = (impl AsRef<str>, impl Into<String> + Clone)> +
Clone,
+>(
+ url: impl AsRef<str>,
+ options: I,
+) -> Result<Box<DynObjectStore>> {
+ let storage_url = url.as_ref();
+
+ if let Ok(url) = Url::parse(storage_url) {
+ let opts = options
+ .clone()
+ .into_iter()
+ .map(|(key, value)| (key.as_ref().to_ascii_lowercase(), value));
+
+ let allow_http: bool = options.into_iter().any(|(key, value)| {
Review Comment:
I think this should probably be made into a config option where relevant, as
opposed to special casing it
##########
object_store/src/lib.rs:
##########
@@ -717,6 +726,139 @@ impl From<Error> for std::io::Error {
}
}
+/// Creates object store from provided url and options
+///
+/// The scheme of the provided url is used to instantiate the store. If the url
+/// scheme is cannot be mapped to a store, [`NotImplemented`] is raised. For
invalid
+/// input, e.g. url with no scheme the default behaviour is to return
+/// [`local::LocalFileSystem`].
+///
+/// # Examples
+///
+/// ```
+///
+/// ```
+pub fn parse_url<
+ I: IntoIterator<Item = (impl AsRef<str>, impl Into<String> + Clone)> +
Clone,
+>(
+ url: impl AsRef<str>,
+ options: I,
+) -> Result<Box<DynObjectStore>> {
+ let storage_url = url.as_ref();
+
+ if let Ok(url) = Url::parse(storage_url) {
+ let opts = options
+ .clone()
+ .into_iter()
+ .map(|(key, value)| (key.as_ref().to_ascii_lowercase(), value));
+
+ let allow_http: bool = options.into_iter().any(|(key, value)| {
+ key.as_ref().to_ascii_lowercase().contains("allow_http")
+ & value.into().eq_ignore_ascii_case("true")
+ });
+
+ match url.scheme() {
+ #[cfg(any(feature = "aws", feature = "aws_profile"))]
+ "s3" | "s3a" => {
+ let opts = opts
Review Comment:
What is the motivation for filtering the options? IMO if you provide an
invalid option it should return an error?
##########
object_store/src/lib.rs:
##########
@@ -705,6 +707,13 @@ pub enum Error {
store
))]
UnknownConfigurationKey { store: &'static str, key: String },
+
+ #[snafu(display(
+ "object_store must be built with feature '{}' to support loading from
'{}'.",
+ feature,
+ url
+ ))]
+ MissingFeature { feature: &'static str, url: String },
Review Comment:
:+1:
##########
object_store/src/lib.rs:
##########
@@ -717,6 +726,139 @@ impl From<Error> for std::io::Error {
}
}
+/// Creates object store from provided url and options
+///
+/// The scheme of the provided url is used to instantiate the store. If the url
+/// scheme is cannot be mapped to a store, [`NotImplemented`] is raised. For
invalid
+/// input, e.g. url with no scheme the default behaviour is to return
+/// [`local::LocalFileSystem`].
+///
+/// # Examples
+///
+/// ```
+///
+/// ```
+pub fn parse_url<
+ I: IntoIterator<Item = (impl AsRef<str>, impl Into<String> + Clone)> +
Clone,
+>(
+ url: impl AsRef<str>,
+ options: I,
+) -> Result<Box<DynObjectStore>> {
+ let storage_url = url.as_ref();
+
+ if let Ok(url) = Url::parse(storage_url) {
Review Comment:
I'd support returning an error instead
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
