kevinjqliu commented on code in PR #2149:
URL: https://github.com/apache/iceberg-python/pull/2149#discussion_r3447654514
##########
pyiceberg/catalog/bigquery_metastore.py:
##########
@@ -334,6 +334,9 @@ def load_view(self, identifier: str | Identifier) -> View:
raise NotImplementedError
@override
+ def rename_view(self, from_identifier: str | Identifier, to_identifier:
str | Identifier) -> None:
+ raise NotImplementedError
+
def load_namespace_properties(self, namespace: str | Identifier) ->
Properties:
Review Comment:
```suggestion
@override
def load_namespace_properties(self, namespace: str | Identifier) ->
Properties:
```
##########
pyiceberg/catalog/rest/__init__.py:
##########
@@ -1471,6 +1474,29 @@ def drop_view(self, identifier: str | Identifier) ->
None:
except HTTPError as exc:
_handle_non_200_response(exc, {404: NoSuchViewError})
+ @retry(**_RETRY_ARGS)
+ def rename_view(self, from_identifier: str | Identifier, to_identifier:
str | Identifier) -> None:
Review Comment:
+1 to adding the override decorator
```suggestion
@override
def rename_view(self, from_identifier: str | Identifier, to_identifier:
str | Identifier) -> None:
```
##########
pyiceberg/catalog/rest/__init__.py:
##########
@@ -1471,6 +1474,29 @@ def drop_view(self, identifier: str | Identifier) ->
None:
except HTTPError as exc:
_handle_non_200_response(exc, {404: NoSuchViewError})
+ @retry(**_RETRY_ARGS)
+ def rename_view(self, from_identifier: str | Identifier, to_identifier:
str | Identifier) -> None:
+ self._check_endpoint(Capability.V1_RENAME_VIEW)
+ payload = {
+ "source": self._split_identifier_for_json(from_identifier),
+ "destination": self._split_identifier_for_json(to_identifier),
+ }
+
+ # Ensure source and destination namespaces exist before rename.
+ source_namespace =
self._split_identifier_for_json(from_identifier)["namespace"]
+ dest_namespace =
self._split_identifier_for_json(to_identifier)["namespace"]
Review Comment:
nit: `self._split_identifier_for_json` is called twice per namespace
i think we can refactor here 😄
##########
pyiceberg/catalog/sql.py:
##########
@@ -784,3 +784,6 @@ def close(self) -> None:
"""
if hasattr(self, "engine"):
self.engine.dispose()
+
+ def rename_view(self, from_identifier: str | Identifier, to_identifier:
str | Identifier) -> None:
Review Comment:
```suggestion
@override
def rename_view(self, from_identifier: str | Identifier, to_identifier:
str | Identifier) -> None:
```
+1
##########
pyiceberg/catalog/bigquery_metastore.py:
##########
@@ -334,6 +334,9 @@ def load_view(self, identifier: str | Identifier) -> View:
raise NotImplementedError
@override
+ def rename_view(self, from_identifier: str | Identifier, to_identifier:
str | Identifier) -> None:
+ raise NotImplementedError
+
def load_namespace_properties(self, namespace: str | Identifier) ->
Properties:
Review Comment:
```suggestion
@override
def load_namespace_properties(self, namespace: str | Identifier) ->
Properties:
```
##########
pyiceberg/catalog/__init__.py:
##########
@@ -744,6 +744,19 @@ def create_view(
ViewAlreadyExistsError: If a view with the name already exists.
"""
+ @abstractmethod
+ def rename_view(self, from_identifier: str | Identifier, to_identifier:
str | Identifier) -> None:
+ """Rename a fully classified view name.
Review Comment:
```suggestion
"""Rename a fully qualified view name.
```
+1
##########
pyiceberg/catalog/bigquery_metastore.py:
##########
@@ -334,6 +334,9 @@ def load_view(self, identifier: str | Identifier) -> View:
raise NotImplementedError
@override
+ def rename_view(self, from_identifier: str | Identifier, to_identifier:
str | Identifier) -> None:
Review Comment:
+1
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
