Skip to content

Commit da28da3

Browse files
rambleraptorclaude
authored andcommitted
Address review comments on rename_view
- Add @OverRide to RestCatalog/SqlCatalog rename_view and restore it on BigQueryMetastoreCatalog.load_namespace_properties - Split identifiers once in RestCatalog.rename_view - Fix "fully classified" typo in the Catalog.rename_view docstring - Add a cross-namespace rename integration test - Drop a stale skip on test_rest_custom_namespace_separator reintroduced by rebase Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 02b6a83 commit da28da3

5 files changed

Lines changed: 34 additions & 12 deletions

File tree

‎pyiceberg/catalog/__init__.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -746,7 +746,7 @@ def create_view(
746746

747747
@abstractmethod
748748
def rename_view(self, from_identifier: str | Identifier, to_identifier: str | Identifier) -> None:
749-
"""Rename a fully classified view name.
749+
"""Rename a fully qualified view name.
750750
751751
Args:
752752
from_identifier (str | Identifier): Existing view identifier.

‎pyiceberg/catalog/bigquery_metastore.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -337,6 +337,7 @@ def load_view(self, identifier: str | Identifier) -> View:
337337
def rename_view(self, from_identifier: str | Identifier, to_identifier: str | Identifier) -> None:
338338
raise NotImplementedError
339339

340+
@override
340341
def load_namespace_properties(self, namespace: str | Identifier) -> Properties:
341342
dataset_name = self.identifier_to_database(namespace, NoSuchNamespaceError)
342343

‎pyiceberg/catalog/rest/__init__.py‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1859,23 +1859,23 @@ def drop_view(self, identifier: str | Identifier) -> None:
18591859
except HTTPError as exc:
18601860
_handle_non_200_response(exc, {404: NoSuchViewError})
18611861

1862+
@override
18621863
@retry(**_RETRY_ARGS)
18631864
def rename_view(self, from_identifier: str | Identifier, to_identifier: str | Identifier) -> None:
18641865
self._check_endpoint(Capability.V1_RENAME_VIEW)
1865-
payload = {
1866-
"source": self._split_identifier_for_json(from_identifier),
1867-
"destination": self._split_identifier_for_json(to_identifier),
1868-
}
1869-
1870-
# Ensure source and destination namespaces exist before rename.
1871-
source_namespace = self._split_identifier_for_json(from_identifier)["namespace"]
1872-
dest_namespace = self._split_identifier_for_json(to_identifier)["namespace"]
1866+
source = self._split_identifier_for_json(from_identifier)
1867+
destination = self._split_identifier_for_json(to_identifier)
18731868

1869+
# Ensure that namespaces exist on source and destination.
1870+
source_namespace = source["namespace"]
18741871
if not self.namespace_exists(source_namespace):
18751872
raise NoSuchNamespaceError(f"Source namespace does not exist: {source_namespace}")
1876-
if not self.namespace_exists(dest_namespace):
1877-
raise NoSuchNamespaceError(f"Destination namespace does not exist: {dest_namespace}")
18781873

1874+
destination_namespace = destination["namespace"]
1875+
if not self.namespace_exists(destination_namespace):
1876+
raise NoSuchNamespaceError(f"Destination namespace does not exist: {destination_namespace}")
1877+
1878+
payload = {"source": source, "destination": destination}
18791879
response = self._session.post(self.url(Endpoints.rename_view), json=payload)
18801880
try:
18811881
response.raise_for_status()

‎pyiceberg/catalog/sql.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -848,5 +848,6 @@ def close(self) -> None:
848848
if hasattr(self, "engine"):
849849
self.engine.dispose()
850850

851+
@override
851852
def rename_view(self, from_identifier: str | Identifier, to_identifier: str | Identifier) -> None:
852853
raise NotImplementedError

‎tests/integration/test_catalog.py‎

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -786,7 +786,27 @@ def test_rest_rename_view(
786786

787787

788788
@pytest.mark.integration
789-
@pytest.mark.skip(reason="Requires Iceberg REST Fixtures 1.11.x")
789+
def test_rest_rename_view_across_namespaces(
790+
rest_catalog: RestCatalog, example_view_metadata_v1: dict[str, Any], database_name: str, view_name: str
791+
) -> None:
792+
to_database_name = f"{database_name}_renamed"
793+
from_identifier = (database_name, view_name)
794+
to_identifier = (to_database_name, view_name)
795+
796+
rest_catalog.create_namespace_if_not_exists(database_name)
797+
rest_catalog.create_namespace_if_not_exists(to_database_name)
798+
view = View(from_identifier, ViewMetadata.model_validate(example_view_metadata_v1))
799+
800+
rest_catalog.create_view(from_identifier, view.schema(), view.current_version())
801+
assert rest_catalog.view_exists(from_identifier)
802+
803+
rest_catalog.rename_view(from_identifier, to_identifier)
804+
805+
assert not rest_catalog.view_exists(from_identifier)
806+
assert rest_catalog.view_exists(to_identifier)
807+
808+
809+
@pytest.mark.integration
790810
def test_rest_custom_namespace_separator(rest_catalog: RestCatalog, table_schema_simple: Schema) -> None:
791811
"""
792812
Tests that the REST catalog correctly picks up the namespace-separator from the config endpoint.

0 commit comments

Comments
 (0)