Skip to content

Commit 3e0f122

Browse files
authored
Add force_path_style S3 config to force path-style addressing (#805)
* Add `force_path_style` S3 config to force path-style addressing Closes #793 * Cleanup * Switch to bool * Add serde default * fix * Fix more tests * fix one more test * fix one last time. * couple more. * last one? !
1 parent 584d4f2 commit 3e0f122

21 files changed

+84
-17
lines changed

docs/docs/icechunk-python/storage.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,7 @@ icechunk.s3_storage(
105105
secret_access_key='minio123',
106106
endpoint_url='http://localhost:9000',
107107
allow_http=True,
108+
force_path_style=True,
108109
```
109110

110111
A few things to note:

icechunk-python/python/icechunk/_icechunk_python.pyi

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ class S3Options:
1212
endpoint_url: str | None = None,
1313
allow_http: bool = False,
1414
anonymous: bool = False,
15+
force_path_style: bool = False,
1516
) -> None:
1617
"""
1718
Create a new `S3Options` object
@@ -26,6 +27,8 @@ class S3Options:
2627
Whether to allow HTTP requests to the storage backend.
2728
anonymous: bool
2829
Whether to use anonymous credentials to the storage backend. When `True`, the s3 requests will not be signed.
30+
force_path_style: bool
31+
Whether to force use of path-style addressing for buckets.
2932
"""
3033

3134
class ObjectStoreConfig:

icechunk-python/python/icechunk/credentials.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -323,6 +323,7 @@ def containers_credentials(
323323
endpoint_url="http://localhost:9000",
324324
allow_http=True,
325325
s3_compatible=True,
326+
force_path_style=True,
326327
)
327328
container = ic.VirtualChunkContainer("s3", "s3://", virtual_store_config)
328329
config.set_virtual_chunk_container(container)

icechunk-python/python/icechunk/storage.py

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,9 +46,15 @@ def s3_store(
4646
allow_http: bool = False,
4747
anonymous: bool = False,
4848
s3_compatible: bool = False,
49+
force_path_style: bool = False,
4950
) -> ObjectStoreConfig.S3Compatible | ObjectStoreConfig.S3:
5051
"""Build an ObjectStoreConfig instance for S3 or S3 compatible object stores."""
51-
options = S3Options(region=region, endpoint_url=endpoint_url, allow_http=allow_http)
52+
options = S3Options(
53+
region=region,
54+
endpoint_url=endpoint_url,
55+
allow_http=allow_http,
56+
force_path_style=force_path_style,
57+
)
5258
return (
5359
ObjectStoreConfig.S3Compatible(options)
5460
if s3_compatible
@@ -70,6 +76,7 @@ def s3_storage(
7076
anonymous: bool | None = None,
7177
from_env: bool | None = None,
7278
get_credentials: Callable[[], S3StaticCredentials] | None = None,
79+
force_path_style: bool = False,
7380
) -> Storage:
7481
"""Create a Storage instance that saves data in S3 or S3 compatible object stores.
7582
@@ -99,6 +106,8 @@ def s3_storage(
99106
Fetch credentials from the operative system environment
100107
get_credentials: Callable[[], S3StaticCredentials] | None
101108
Use this function to get and refresh object store credentials
109+
force_path_style: bool
110+
Whether to force using path-style addressing for buckets
102111
"""
103112

104113
credentials = s3_credentials(
@@ -110,7 +119,12 @@ def s3_storage(
110119
from_env=from_env,
111120
get_credentials=get_credentials,
112121
)
113-
options = S3Options(region=region, endpoint_url=endpoint_url, allow_http=allow_http)
122+
options = S3Options(
123+
region=region,
124+
endpoint_url=endpoint_url,
125+
allow_http=allow_http,
126+
force_path_style=force_path_style,
127+
)
114128
return Storage.new_s3(
115129
config=options,
116130
bucket=bucket,
@@ -132,6 +146,7 @@ def s3_object_store_storage(
132146
expires_after: datetime | None = None,
133147
anonymous: bool | None = None,
134148
from_env: bool | None = None,
149+
force_path_style: bool = False,
135150
) -> Storage:
136151
credentials = s3_credentials(
137152
access_key_id=access_key_id,
@@ -142,7 +157,12 @@ def s3_object_store_storage(
142157
from_env=from_env,
143158
get_credentials=None,
144159
)
145-
options = S3Options(region=region, endpoint_url=endpoint_url, allow_http=allow_http)
160+
options = S3Options(
161+
region=region,
162+
endpoint_url=endpoint_url,
163+
allow_http=allow_http,
164+
force_path_style=force_path_style,
165+
)
146166
return Storage.new_s3_object_store(
147167
config=options,
148168
bucket=bucket,

icechunk-python/src/config.rs

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -351,29 +351,33 @@ pub struct PyS3Options {
351351
pub allow_http: bool,
352352
#[pyo3(get, set)]
353353
pub anonymous: bool,
354+
#[pyo3(get, set)]
355+
pub force_path_style: bool,
354356
}
355357

356358
#[pymethods]
357359
impl PyS3Options {
358360
#[new]
359-
#[pyo3(signature = ( region=None, endpoint_url=None, allow_http=false, anonymous=false))]
361+
#[pyo3(signature = ( region=None, endpoint_url=None, allow_http=false, anonymous=false, force_path_style=false))]
360362
pub(crate) fn new(
361363
region: Option<String>,
362364
endpoint_url: Option<String>,
363365
allow_http: bool,
364366
anonymous: bool,
367+
force_path_style: bool,
365368
) -> Self {
366-
Self { region, endpoint_url, allow_http, anonymous }
369+
Self { region, endpoint_url, allow_http, anonymous, force_path_style }
367370
}
368371

369372
pub fn __repr__(&self) -> String {
370373
// TODO: escape
371374
format!(
372-
r#"S3Options(region={region}, endpoint_url={url}, allow_http={http}, anonymous={anon})"#,
375+
r#"S3Options(region={region}, endpoint_url={url}, allow_http={http}, anonymous={anon}, force_path_style={force_path_style})"#,
373376
region = format_option(self.region.as_ref()),
374377
url = format_option(self.endpoint_url.as_ref()),
375378
http = format_bool(self.allow_http),
376379
anon = format_bool(self.anonymous),
380+
force_path_style = format_bool(self.force_path_style),
377381
)
378382
}
379383
}
@@ -385,6 +389,7 @@ impl From<&PyS3Options> for S3Options {
385389
endpoint_url: options.endpoint_url.clone(),
386390
allow_http: options.allow_http,
387391
anonymous: options.anonymous,
392+
force_path_style: options.force_path_style,
388393
}
389394
}
390395
}
@@ -396,6 +401,7 @@ impl From<S3Options> for PyS3Options {
396401
endpoint_url: value.endpoint_url,
397402
allow_http: value.allow_http,
398403
anonymous: value.anonymous,
404+
force_path_style: value.force_path_style,
399405
}
400406
}
401407
}

icechunk-python/tests/run_xarray_backends_tests.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@ def create_zarr_target(self) -> Generator[IcechunkStore]:
8585
s3_storage(
8686
endpoint_url="http://localhost:9000",
8787
allow_http=True,
88+
force_path_style=True,
8889
region="us-east-1",
8990
bucket="testbucket",
9091
prefix="python-xarray-test__" + str(time.time()),

icechunk-python/tests/test_can_read_old.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ def mk_repo(create: bool) -> ic.Repository:
3333
endpoint_url="http://localhost:9000",
3434
allow_http=True,
3535
s3_compatible=True,
36+
force_path_style=True,
3637
)
3738
container = ic.VirtualChunkContainer("s3", "s3://", virtual_store_config)
3839
config.set_virtual_chunk_container(container)

icechunk-python/tests/test_concurrency.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,7 @@ async def test_thread_concurrency() -> None:
120120
region="us-east-1",
121121
endpoint_url="http://localhost:9000",
122122
allow_http=True,
123+
force_path_style=True,
123124
s3_compatible=True,
124125
)
125126
container = icechunk.VirtualChunkContainer("s3", "s3://", store_config)
@@ -133,6 +134,7 @@ async def test_thread_concurrency() -> None:
133134
region="us-east-1",
134135
endpoint_url="http://localhost:9000",
135136
allow_http=True,
137+
force_path_style=True,
136138
bucket="testbucket",
137139
prefix="multithreaded-test__" + str(time.time()),
138140
access_key_id="minio123",

icechunk-python/tests/test_credentials.py

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ def test_refreshable_credentials_grant_access() -> None:
2626
region="us-east-1",
2727
endpoint_url="http://localhost:9000",
2828
allow_http=True,
29+
force_path_style=True,
2930
bucket="testbucket",
3031
prefix="this-repo-does-not-exist",
3132
get_credentials=get_good_credentials,
@@ -34,6 +35,7 @@ def test_refreshable_credentials_grant_access() -> None:
3435
region="us-east-1",
3536
endpoint_url="http://localhost:9000",
3637
allow_http=True,
38+
force_path_style=True,
3739
bucket="testbucket",
3840
prefix="this-repo-does-not-exist",
3941
get_credentials=get_bad_credentials,
@@ -68,7 +70,10 @@ def test_refreshable_credentials_errors() -> None:
6870

6971
st = Storage.new_s3(
7072
config=S3Options(
71-
region="us-east-1", endpoint_url="http://localhost:9000", allow_http=True
73+
region="us-east-1",
74+
endpoint_url="http://localhost:9000",
75+
allow_http=True,
76+
force_path_style=True,
7277
),
7378
bucket="testbucket",
7479
prefix="this-repo-does-not-exist",
@@ -80,7 +85,10 @@ def test_refreshable_credentials_errors() -> None:
8085

8186
st = Storage.new_s3(
8287
config=S3Options(
83-
region="us-east-1", endpoint_url="http://localhost:9000", allow_http=True
88+
region="us-east-1",
89+
endpoint_url="http://localhost:9000",
90+
allow_http=True,
91+
force_path_style=True,
8492
),
8593
bucket="testbucket",
8694
prefix="this-repo-does-not-exist",
@@ -121,6 +129,7 @@ def test_s3_refreshable_credentials_refresh(tmp_path: Path) -> None:
121129
region="us-east-1",
122130
endpoint_url="http://localhost:9000",
123131
allow_http=True,
132+
force_path_style=True,
124133
bucket="testbucket",
125134
prefix="this-repo-does-not-exist",
126135
get_credentials=creds_obj,

icechunk-python/tests/test_distributed_writers.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ def mk_repo(use_object_store: bool = False) -> icechunk.Repository:
2727
storage = s3_object_store_storage(
2828
endpoint_url="http://localhost:9000",
2929
allow_http=True,
30+
force_path_style=True,
3031
region="us-east-1",
3132
bucket="testbucket",
3233
prefix="python-distributed-writers-test__" + str(time.time()),
@@ -37,6 +38,7 @@ def mk_repo(use_object_store: bool = False) -> icechunk.Repository:
3738
storage = s3_storage(
3839
endpoint_url="http://localhost:9000",
3940
allow_http=True,
41+
force_path_style=True,
4042
region="us-east-1",
4143
bucket="testbucket",
4244
prefix="python-distributed-writers-test__" + str(time.time()),

0 commit comments

Comments
 (0)