Skip to content

Commit 7f7823a

Browse files
lizardoluislizardoluis
authored
Fix object-store with_config
This commit moves the parsing of the config keys into the with_config method of the ObjectStoreBuilder. This is to avoid issues where multiple providers use the same config keys and it hence can lead to a mismatch between ObjectStoreBuilder and ConfigKey
2 parents 4ed5209 + b4041ba commit 7f7823a

File tree

7 files changed

+82
-39
lines changed

7 files changed

+82
-39
lines changed

‎catalogs/iceberg-file-catalog/src/lib.rs‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -713,14 +713,19 @@ pub mod tests {
713713
let localstack_port = localstack.get_host_port_ipv4(4566).await.unwrap();
714714

715715
let object_store = ObjectStoreBuilder::s3()
716-
.with_config("aws_access_key_id".parse().unwrap(), "user")
717-
.with_config("aws_secret_access_key".parse().unwrap(), "password")
716+
.with_config("aws_access_key_id", "user")
717+
.unwrap()
718+
.with_config("aws_secret_access_key", "password")
719+
.unwrap()
718720
.with_config(
719-
"endpoint".parse().unwrap(),
721+
"endpoint",
720722
format!("http://{localstack_host}:{localstack_port}"),
721723
)
722-
.with_config("region".parse().unwrap(), "us-east-1")
723-
.with_config("allow_http".parse().unwrap(), "true");
724+
.unwrap()
725+
.with_config("region", "us-east-1")
726+
.unwrap()
727+
.with_config("allow_http", "true")
728+
.unwrap();
724729
// let object_store = ObjectStoreBuilder::memory();
725730

726731
let iceberg_catalog: Arc<dyn Catalog> = Arc::new(

‎catalogs/iceberg-glue-catalog/src/lib.rs‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1063,14 +1063,19 @@ pub mod tests {
10631063
let localstack_port = localstack.get_host_port_ipv4(4566).await.unwrap();
10641064

10651065
let object_store = ObjectStoreBuilder::s3()
1066-
.with_config("aws_access_key_id".parse().unwrap(), "user")
1067-
.with_config("aws_secret_access_key".parse().unwrap(), "password")
1066+
.with_config("aws_access_key_id", "user")
1067+
.unwrap()
1068+
.with_config("aws_secret_access_key", "password")
1069+
.unwrap()
10681070
.with_config(
1069-
"endpoint".parse().unwrap(),
1071+
"endpoint",
10701072
format!("http://{localstack_host}:{localstack_port}"),
10711073
)
1072-
.with_config("region".parse().unwrap(), "us-east-1")
1073-
.with_config("allow_http".parse().unwrap(), "true");
1074+
.unwrap()
1075+
.with_config("region", "us-east-1")
1076+
.unwrap()
1077+
.with_config("allow_http", "true")
1078+
.unwrap();
10741079

10751080
// let object_store = ObjectStoreBuilder::memory();
10761081
let iceberg_catalog: Arc<dyn Catalog> =

‎catalogs/iceberg-rest-catalog/src/catalog.rs‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -726,14 +726,19 @@ pub mod tests {
726726
let rest_port = rest.get_host_port_ipv4(8181).await.unwrap();
727727

728728
let object_store = ObjectStoreBuilder::s3()
729-
.with_config("aws_access_key_id".parse().unwrap(), "user")
730-
.with_config("aws_secret_access_key".parse().unwrap(), "password")
729+
.with_config("aws_access_key_id", "user")
730+
.unwrap()
731+
.with_config("aws_secret_access_key", "password")
732+
.unwrap()
731733
.with_config(
732-
"endpoint".parse().unwrap(),
734+
"endpoint",
733735
format!("http://{localstack_host}:{localstack_port}"),
734736
)
735-
.with_config("region".parse().unwrap(), "us-east-1")
736-
.with_config("allow_http".parse().unwrap(), "true");
737+
.unwrap()
738+
.with_config("region", "us-east-1")
739+
.unwrap()
740+
.with_config("allow_http", "true")
741+
.unwrap();
737742

738743
let iceberg_catalog = Arc::new(RestCatalog::new(
739744
None,

‎catalogs/iceberg-sql-catalog/src/lib.rs‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -829,14 +829,19 @@ pub mod tests {
829829
let localstack_port = localstack.get_host_port_ipv4(4566).await.unwrap();
830830

831831
let object_store = ObjectStoreBuilder::s3()
832-
.with_config("aws_access_key_id".parse().unwrap(), "user")
833-
.with_config("aws_secret_access_key".parse().unwrap(), "password")
832+
.with_config("aws_access_key_id", "user")
833+
.unwrap()
834+
.with_config("aws_secret_access_key", "password")
835+
.unwrap()
834836
.with_config(
835-
"endpoint".parse().unwrap(),
837+
"endpoint",
836838
format!("http://{localstack_host}:{localstack_port}"),
837839
)
838-
.with_config("region".parse().unwrap(), "us-east-1")
839-
.with_config("allow_http".parse().unwrap(), "true");
840+
.unwrap()
841+
.with_config("region", "us-east-1")
842+
.unwrap()
843+
.with_config("allow_http", "true")
844+
.unwrap();
840845

841846
let iceberg_catalog = Arc::new(
842847
SqlCatalog::new(

‎datafusion_iceberg/tests/empty_insert.rs‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -44,14 +44,19 @@ pub async fn test_empty_insert() {
4444
let localstack_port = localstack.get_host_port_ipv4(4566).await.unwrap();
4545

4646
let object_store = ObjectStoreBuilder::s3()
47-
.with_config("aws_access_key_id".parse().unwrap(), "user")
48-
.with_config("aws_secret_access_key".parse().unwrap(), "password")
47+
.with_config("aws_access_key_id", "user")
48+
.unwrap()
49+
.with_config("aws_secret_access_key", "password")
50+
.unwrap()
4951
.with_config(
50-
"endpoint".parse().unwrap(),
52+
"endpoint",
5153
format!("http://{localstack_host}:{localstack_port}"),
5254
)
53-
.with_config("region".parse().unwrap(), "us-east-1")
54-
.with_config("allow_http".parse().unwrap(), "true");
55+
.unwrap()
56+
.with_config("region", "us-east-1")
57+
.unwrap()
58+
.with_config("allow_http", "true")
59+
.unwrap();
5560

5661
let iceberg_catalog_list = Arc::new(
5762
SqlCatalogList::new("sqlite://", object_store.clone())

‎datafusion_iceberg/tests/integration_trino.rs‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -185,14 +185,19 @@ async fn integration_trino_rest() {
185185
.unwrap();
186186

187187
let object_store = ObjectStoreBuilder::s3()
188-
.with_config("aws_access_key_id".parse().unwrap(), "user")
189-
.with_config("aws_secret_access_key".parse().unwrap(), "password")
188+
.with_config("aws_access_key_id", "user")
189+
.unwrap()
190+
.with_config("aws_secret_access_key", "password")
191+
.unwrap()
190192
.with_config(
191-
"endpoint".parse().unwrap(),
193+
"endpoint",
192194
format!("http://{localstack_host}:{localstack_port}"),
193195
)
194-
.with_config("region".parse().unwrap(), "us-east-1")
195-
.with_config("allow_http".parse().unwrap(), "true");
196+
.unwrap()
197+
.with_config("region", "us-east-1")
198+
.unwrap()
199+
.with_config("allow_http", "true")
200+
.unwrap();
196201

197202
let catalog = Arc::new(RestCatalog::new(
198203
None,

‎iceberg-rust/src/object_store/mod.rs‎

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -149,18 +149,31 @@ impl ObjectStoreBuilder {
149149
ObjectStoreBuilder::Memory(Arc::new(InMemory::new()))
150150
}
151151
/// Set config value for builder
152-
pub fn with_config(self, key: ConfigKey, value: impl Into<String>) -> Self {
153-
match (self, key) {
154-
(ObjectStoreBuilder::Azure(azure), ConfigKey::Azure(key)) => {
155-
ObjectStoreBuilder::Azure(Box::new(azure.with_config(key, value)))
152+
pub fn with_config(
153+
self,
154+
key: impl Into<String>,
155+
value: impl Into<String>,
156+
) -> Result<Self, Error> {
157+
match self {
158+
ObjectStoreBuilder::Azure(azure) => {
159+
let key: AzureConfigKey = key.into().parse()?;
160+
Ok(ObjectStoreBuilder::Azure(Box::new(
161+
azure.with_config(key, value),
162+
)))
156163
}
157-
(ObjectStoreBuilder::S3(aws), ConfigKey::AWS(key)) => {
158-
ObjectStoreBuilder::S3(Box::new(aws.with_config(key, value)))
164+
ObjectStoreBuilder::S3(aws) => {
165+
let key: AmazonS3ConfigKey = key.into().parse()?;
166+
Ok(ObjectStoreBuilder::S3(Box::new(
167+
aws.with_config(key, value),
168+
)))
159169
}
160-
(ObjectStoreBuilder::GCS(gcs), ConfigKey::GCS(key)) => {
161-
ObjectStoreBuilder::GCS(Box::new(gcs.with_config(key, value)))
170+
ObjectStoreBuilder::GCS(gcs) => {
171+
let key: GoogleConfigKey = key.into().parse()?;
172+
Ok(ObjectStoreBuilder::GCS(Box::new(
173+
gcs.with_config(key, value),
174+
)))
162175
}
163-
(x, _) => x,
176+
x => Ok(x),
164177
}
165178
}
166179
/// Create objectstore from template

0 commit comments

Comments
 (0)