PR comments

Author: rambleraptor

pyiceberg/catalog/rest/__init__.py

@@ -320,7 +320,6 @@ class ListViewsResponse(IcebergBaseModel):
 class RestCatalog(Catalog):
     uri: str
     _session: Session
-    _supported_endpoints: set[Endpoint]
     _namespace_separator: str
 
     def __init__(self, name: str, **properties: str):
@@ -337,6 +336,8 @@ def __init__(self, name: str, **properties: str):
         self._fetch_config()
         self._session = self._create_session()
         separator_from_properties = self.properties.get(NAMESPACE_SEPARATOR_PROPERTY, DEFAULT_NAMESPACE_SEPARATOR)
+        if not separator_from_properties:
+            raise ValueError("Namespace separator cannot be an empty string")
         self._namespace_separator = unquote(separator_from_properties)
 
     def _create_session(self) -> Session:
@@ -534,8 +535,6 @@ def _identifier_to_validated_tuple(self, identifier: str | Identifier) -> Identi
     def _split_identifier_for_path(
         self, identifier: str | Identifier | TableIdentifier, kind: IdentifierKind = IdentifierKind.TABLE
     ) -> Properties:
-        from urllib.parse import quote
-
         if isinstance(identifier, TableIdentifier):
             return {
                 "namespace": self._encode_namespace_path(tuple(identifier.namespace.root)),
@@ -933,7 +932,7 @@ def list_namespaces(self, namespace: str | Identifier = ()) -> list[Identifier]:
         namespace_tuple = self.identifier_to_tuple(namespace)
         response = self._session.get(
             self.url(
-                f"{Endpoints.list_namespaces}?parent={self._namespace_separator.join(namespace_tuple)}"
+                f"{Endpoints.list_namespaces}?parent={self._encode_namespace_path(namespace_tuple)}"
                 if namespace_tuple
                 else Endpoints.list_namespaces
             ),
@@ -983,14 +982,6 @@ def namespace_exists(self, namespace: str | Identifier) -> bool:
         namespace_tuple = self._check_valid_namespace_identifier(namespace)
         namespace = self._encode_namespace_path(namespace_tuple)
 
-        # fallback in order to work with older rest catalog implementations
-        if Capability.V1_NAMESPACE_EXISTS not in self._supported_endpoints:
-            try:
-                self.load_namespace_properties(namespace_tuple)
-                return True
-            except NoSuchNamespaceError:
-                return False
-
         response = self._session.head(self.url(Endpoints.namespace_exists, namespace=namespace))
 
         if response.status_code == 404:

tests/catalog/test_rest.py

@@ -1921,8 +1921,6 @@ def test_rest_catalog_with_google_credentials_path(
     actual_headers = history[0].headers
     assert actual_headers["Authorization"] == expected_auth_header
 
-    assert actual_headers["Authorization"] == expected_auth_header
-
 
 def test_custom_namespace_separator(rest_mock: Mocker) -> None:
     custom_separator = "-"
@@ -1939,7 +1937,7 @@ def test_custom_namespace_separator(rest_mock: Mocker) -> None:
     rest_mock.get(
         f"{TEST_URI}v1/namespaces/{expected_url_path_segment}",
         json={"namespace": [namespace_part1, namespace_part2], "properties": {"prop": "yes"}},
-        status_code=204,
+        status_code=200,
         request_headers=TEST_HEADERS,
     )