fix(fsspec): parse S3 virtual addressing as a boolean

pyiceberg/io/fsspec.py

@@ -201,7 +201,7 @@ def _s3(properties: Properties) -> AbstractFileSystem:
     if request_timeout := properties.get(S3_REQUEST_TIMEOUT):
         config_kwargs["read_timeout"] = float(request_timeout)
 
-    if _force_virtual_addressing := properties.get(S3_FORCE_VIRTUAL_ADDRESSING):
+    if property_as_bool(properties, S3_FORCE_VIRTUAL_ADDRESSING, False):
         config_kwargs["s3"] = {"addressing_style": "virtual"}
 
     if s3_anonymous := properties.get(S3_ANONYMOUS):

pyiceberg/utils/properties.py

@@ -54,13 +54,13 @@ def property_as_float(
 
 
 def property_as_bool(
-    properties: dict[str, str],
+    properties: Properties,
     property_name: str,
     default: bool,
 ) -> bool:
-    if value := properties.get(property_name):
+    if (value := properties.get(property_name)) not in (None, ""):
         try:
-            return strtobool(value)
+            return strtobool(str(value))
         except ValueError as e:
             raise ValueError(f"Could not parse table property {property_name} to a boolean: {value}") from e
     return default

tests/io/test_fsspec.py

@@ -320,9 +320,20 @@ def test_fsspec_s3_session_properties() -> None:
         )
 
 
-def test_fsspec_s3_session_properties_force_virtual_addressing() -> None:
+@pytest.mark.parametrize(
+    ("force_virtual_addressing", "config_kwargs"),
+    [
+        ("true", {"s3": {"addressing_style": "virtual"}}),
+        ("false", {}),
+        (True, {"s3": {"addressing_style": "virtual"}}),
+        (False, {}),
+    ],
+)
+def test_fsspec_s3_session_properties_force_virtual_addressing(
+    force_virtual_addressing: str | bool, config_kwargs: Properties
+) -> None:
     session_properties: Properties = {
-        "s3.force-virtual-addressing": True,
+        "s3.force-virtual-addressing": force_virtual_addressing,
         "s3.endpoint": "http://localhost:9000",
         "s3.access-key-id": "admin",
         "s3.secret-access-key": "password",
@@ -346,7 +357,7 @@ def test_fsspec_s3_session_properties_force_virtual_addressing() -> None:
                 "region_name": "us-east-1",
                 "aws_session_token": "s3.session-token",
             },
-            config_kwargs={"s3": {"addressing_style": "virtual"}},
+            config_kwargs=config_kwargs,
         )
 
 

tests/utils/test_properties.py

@@ -67,14 +67,23 @@ def test_property_as_float_with_invalid_value() -> None:
         assert "Could not parse table property some_float_prop to a float: invalid" in str(exc.value)
 
 
-def test_property_as_bool() -> None:
+@pytest.mark.parametrize(
+    ("value", "expected"),
+    [
+        ("True", True),
+        ("False", False),
+        (True, True),
+        (False, False),
+    ],
+)
+def test_property_as_bool(value: str | bool, expected: bool) -> None:
     properties = {
-        "bool": "True",
+        "bool": value,
     }
 
-    assert property_as_bool(properties, "bool", default=False) is True
+    assert property_as_bool(properties, "bool", default=not expected) is expected
     assert property_as_bool(properties, "missing", default=False) is False
-    assert property_as_float(properties, "missing") is None
+    assert property_as_bool(properties, "missing", default=True) is True
 
 
 def test_property_as_bool_with_invalid_value() -> None: