Skip to content

Commit

Permalink
fix: Allow HTTP object store for json/bson etc.
Browse files Browse the repository at this point in the history
Resolves part of #2756

Signed-off-by: Vaibhav <vrongmeal@gmail.com>
  • Loading branch information
vrongmeal committed Mar 19, 2024
1 parent 589b9e3 commit f55cd5d
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 18 deletions.
5 changes: 2 additions & 3 deletions crates/datasources/src/lake/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ use protogen::metastore::types::options::StorageOptions;
use crate::common::url::{DatasourceUrl, DatasourceUrlType};
use crate::object_store::azure::AzureStoreAccess;
use crate::object_store::gcs::GcsStoreAccess;
use crate::object_store::http::HttpStoreAccess;
use crate::object_store::local::LocalStoreAccess;
use crate::object_store::s3::S3StoreAccess;
use crate::object_store::ObjStoreAccess;
Expand Down Expand Up @@ -101,9 +102,7 @@ pub fn storage_options_into_store_access(
}))
}
DatasourceUrlType::File => Ok(Arc::new(LocalStoreAccess)),
DatasourceUrlType::Http => {
Err(LakeStorageOptionsError::UnsupportedObjectStore(url.clone()))
}
DatasourceUrlType::Http => Ok(Arc::new(HttpStoreAccess { url: url.as_url()? })),
}
}

Expand Down
24 changes: 14 additions & 10 deletions crates/datasources/src/object_store/http.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use datafusion::execution::context::SessionState;
use datafusion::execution::object_store::ObjectStoreUrl;
use object_store::http::HttpBuilder;
use object_store::path::Path as ObjectStorePath;
use object_store::{ObjectMeta, ObjectStore};
use object_store::{ClientConfigKey, ObjectMeta, ObjectStore};
use url::Url;

use super::glob_util::ResolvedPattern;
Expand Down Expand Up @@ -43,13 +43,17 @@ impl ObjStoreAccess for HttpStoreAccess {
// To make path part of URL we make it a '/'.
.replace('/', "__slash__")
// TODO: Add more characters which might be invalid for domain.
.replace('%', "__percent__");
.replace('%', "__percent__")
.replace('?', "__question__")
.replace('=', "__equal__");

Ok(ObjectStoreUrl::parse(u)?)
}

fn create_store(&self) -> Result<Arc<dyn ObjectStore>> {
let builder = HttpBuilder::new().with_url(self.url.to_string());
let builder = HttpBuilder::new()
.with_url(self.url.to_string())
.with_config(ClientConfigKey::AllowHttp, "true");
let build = builder.build()?;
Ok(Arc::new(build))
}
Expand Down Expand Up @@ -94,17 +98,17 @@ impl ObjStoreAccess for HttpStoreAccess {
}
// reqwest doesn't check the content length header, instead looks at the contents
// See: https://github.com/seanmonstar/reqwest/issues/843
let len: u64 = res
let len = res
.headers()
.get("Content-Length")
.and_then(|v| v.to_str().ok())
.and_then(|v| v.parse().ok())
.unwrap_or_else(|| res.content_length().unwrap_or(0));
if len == 0 {
return Err(ObjectStoreSourceError::Static(
.and_then(|v| v.parse::<u64>().ok())
.or_else(|| res.content_length())
// TODO: Download the contents of the file and handle using local
// store (maybe outside of HTTP store).
.ok_or(ObjectStoreSourceError::Static(
"Missing content-length header",
));
}
))?;

Ok(ObjectMeta {
location: location.clone(),
Expand Down
6 changes: 1 addition & 5 deletions crates/sqlbuiltins/src/functions/table/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,7 @@ pub fn table_location_and_opts(
let mut storage_options = StorageOptions::default();
match (source_url.datasource_url_type(), maybe_cred_opts) {
(DatasourceUrlType::File, None) => {} // no options fine in this case
(DatasourceUrlType::Http, None) => {} // no options fine in this case
(DatasourceUrlType::File, _) => {
return Err(ExtensionError::String(
"Credentials incorrectly provided when accessing local delta table".to_string(),
Expand Down Expand Up @@ -224,11 +225,6 @@ pub fn table_location_and_opts(
creds.access_key,
);
}
(DatasourceUrlType::Http, _) => {
return Err(ExtensionError::String(
"Accessing delta tables over http not supported".to_string(),
))
}
(datasource, creds) => {
return Err(ExtensionError::String(format!(
"Invalid credentials for {datasource}, got {} creds",
Expand Down

0 comments on commit f55cd5d

Please sign in to comment.