diff --git a/.gitignore b/.gitignore index 547d6a4a5..810a63b5f 100644 --- a/.gitignore +++ b/.gitignore @@ -16,7 +16,6 @@ benchmarks/artifacts/ /deps .vscode/ -.helix/ # Spark jar for iceberg data generation *.jar diff --git a/crates/glaredb/src/commands.rs b/crates/glaredb/src/commands.rs index e89955ff4..8adab294e 100644 --- a/crates/glaredb/src/commands.rs +++ b/crates/glaredb/src/commands.rs @@ -252,7 +252,7 @@ impl RunCommand for MetastoreArgs { (Some(bucket), Some(service_account_path), None) => { let service_account_key = std::fs::read_to_string(service_account_path)?; StorageConfig::Gcs { - bucket, + bucket: Some(bucket), service_account_key, } } diff --git a/crates/object_store_util/src/conf.rs b/crates/object_store_util/src/conf.rs index ff4979f59..60044a057 100644 --- a/crates/object_store_util/src/conf.rs +++ b/crates/object_store_util/src/conf.rs @@ -18,16 +18,16 @@ pub enum StorageConfig { secret_access_key: String, region: Option, endpoint: Option, - bucket: String, + bucket: Option, }, Gcs { service_account_key: String, - bucket: String, + bucket: Option, }, Azure { account_name: String, access_key: String, - container_name: String, + container_name: Option, }, Local { path: PathBuf, @@ -49,7 +49,6 @@ impl StorageConfig { let mut builder = AmazonS3Builder::new() .with_access_key_id(access_key_id) .with_secret_access_key(secret_access_key) - .with_bucket_name(bucket) .with_region(region.clone().unwrap_or_default()); if let Some(endpoint) = endpoint { @@ -67,15 +66,22 @@ impl StorageConfig { } } + if let Some(bucket) = bucket { + builder = builder.with_bucket_name(bucket); + } + Arc::new(builder.build()?) } StorageConfig::Gcs { service_account_key, bucket, } => { - let builder = GoogleCloudStorageBuilder::new() - .with_service_account_key(service_account_key) - .with_bucket_name(bucket); + let mut builder = + GoogleCloudStorageBuilder::new().with_service_account_key(service_account_key); + + if let Some(bucket) = bucket { + builder = builder.with_bucket_name(bucket); + } Arc::new(builder.build()?) } @@ -84,10 +90,13 @@ impl StorageConfig { access_key, container_name, } => { - let builder = MicrosoftAzureBuilder::new() + let mut builder = MicrosoftAzureBuilder::new() .with_account(account_name) - .with_access_key(access_key) - .with_container_name(container_name); + .with_access_key(access_key); + + if let Some(container_name) = container_name { + builder = builder.with_container_name(container_name); + } Arc::new(builder.build()?) } diff --git a/crates/sqlexec/src/engine.rs b/crates/sqlexec/src/engine.rs index 7bf5481dc..7fcf9e7b1 100644 --- a/crates/sqlexec/src/engine.rs +++ b/crates/sqlexec/src/engine.rs @@ -114,8 +114,7 @@ impl EngineStorageConfig { let bucket = opts .get("bucket") .cloned() - .or(url.host_str().map(|h| h.to_string())) - .ok_or(ExecError::MissingBucketName)?; + .or(url.host_str().map(|h| h.to_string())); EngineStorageConfig { location: url.clone(), @@ -157,8 +156,6 @@ impl EngineStorageConfig { } } - let bucket = bucket.ok_or(ExecError::MissingBucketName)?; - EngineStorageConfig { location: url.clone(), conf: StorageConfig::S3 { @@ -189,8 +186,7 @@ impl EngineStorageConfig { let container_name = opts .get("container_name") .cloned() - .or(url.host_str().map(|h| h.to_string())) - .ok_or(ExecError::MissingBucketName)?; + .or(url.host_str().map(|h| h.to_string())); EngineStorageConfig { location: url.clone(), @@ -231,15 +227,21 @@ impl EngineStorageConfig { .map_err(DatasourceCommonError::from)?, conf: StorageConfig::Gcs { service_account_key, - bucket, + bucket: Some(bucket), }, } } + // Expected gcs config opts for the session. + (StorageConfig::Gcs { bucket: None, .. }, None) => { + return Err(ExecError::InvalidStorageConfig( + "Missing bucket on session configuration", + )) + } (_, Some(_)) => EngineStorageConfig { location: Url::parse("memory://").map_err(DatasourceCommonError::from)?, conf: StorageConfig::Memory, }, - (_, None) => self.clone(), + _ => self.clone(), }) } @@ -565,14 +567,14 @@ mod tests { secret_access_key, region: None, endpoint: None, - bucket: "some-bucket".to_string(), + bucket: Some("some-bucket".to_string()), } ); let merged_conf = conf.with_session_config(&SessionStorageConfig { gcs_bucket: Some("my-other-bucket".to_string()), })?; - assert_eq!(merged_conf.conf, StorageConfig::Memory); + assert_eq!(merged_conf.conf, StorageConfig::Memory,); Ok(()) } } diff --git a/crates/sqlexec/src/errors.rs b/crates/sqlexec/src/errors.rs index 93bca702f..6b48da547 100644 --- a/crates/sqlexec/src/errors.rs +++ b/crates/sqlexec/src/errors.rs @@ -124,9 +124,6 @@ pub enum ExecError { #[error("Invalid URL for remote execution: {0}")] InvalidRemoteExecUrl(String), - #[error("Missing bucket name: required for storage configuration")] - MissingBucketName, - #[error(transparent)] DatasourceDebug(#[from] datasources::debug::errors::DebugError),