Revert "chore: Make bucket name required in storage config" (#2337)

.gitignore

@@ -16,7 +16,6 @@ benchmarks/artifacts/
 /deps
 
 .vscode/
-.helix/
 
 # Spark jar for iceberg data generation
 *.jar

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,
                 }
             }

crates/object_store_util/src/conf.rs

@@ -18,16 +18,16 @@ pub enum StorageConfig {
         secret_access_key: String,
         region: Option<String>,
         endpoint: Option<String>,
-        bucket: String,
+        bucket: Option<String>,
     },
     Gcs {
         service_account_key: String,
-        bucket: String,
+        bucket: Option<String>,
     },
     Azure {
         account_name: String,
         access_key: String,
-        container_name: String,
+        container_name: Option<String>,
     },
     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()?)
             }

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(())
     }
 }

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),