ebyhr commented on code in PR #3288:
URL: https://github.com/apache/iceberg-python/pull/3288#discussion_r3151187652
##########
pyiceberg/catalog/rest/__init__.py:
##########
@@ -1312,6 +1319,27 @@ def view_exists(self, identifier: str | Identifier) ->
bool:
return False
+ @retry(**_RETRY_ARGS)
+ def register_view(self, identifier: str | Identifier, metadata_location:
str) -> View:
+ self._check_endpoint(Capability.V1_REGISTER_VIEW)
+ namespace_and_view = self._split_identifier_for_path(identifier)
+ request = RegisterViewRequest(
+ name=self._identifier_to_validated_tuple(identifier)[-1],
+ metadata_location=metadata_location,
+ )
+ serialized_json = request.model_dump_json().encode(UTF8)
+ response = self._session.post(
+ self.url(Endpoints.register_view,
namespace=namespace_and_view["namespace"]),
+ data=serialized_json,
+ )
+ try:
+ response.raise_for_status()
+ except HTTPError as exc:
+ _handle_non_200_response(exc, {409: ViewAlreadyExistsError})
Review Comment:
The `ViewAlreadyExistsError` in the YAML file is just an example. The actual
exception type may differ in each REST catalog implementation.
If we want to distinguish between tables and views, we should use
`self.table_exists` instead of relying on an exception-driven approach, as Drew
already said.
##########
pyiceberg/catalog/rest/__init__.py:
##########
@@ -1312,6 +1319,27 @@ def view_exists(self, identifier: str | Identifier) ->
bool:
return False
+ @retry(**_RETRY_ARGS)
+ def register_view(self, identifier: str | Identifier, metadata_location:
str) -> View:
+ self._check_endpoint(Capability.V1_REGISTER_VIEW)
+ namespace_and_view = self._split_identifier_for_path(identifier)
+ request = RegisterViewRequest(
+ name=self._identifier_to_validated_tuple(identifier)[-1],
+ metadata_location=metadata_location,
+ )
+ serialized_json = request.model_dump_json().encode(UTF8)
+ response = self._session.post(
+ self.url(Endpoints.register_view,
namespace=namespace_and_view["namespace"]),
+ data=serialized_json,
+ )
+ try:
+ response.raise_for_status()
+ except HTTPError as exc:
+ _handle_non_200_response(exc, {409: ViewAlreadyExistsError})
Review Comment:
> Wonder if here we can catch a TableAlreadyExists if there is a table
already registered with the same identifier?
The `ViewAlreadyExistsError` in the YAML file is just an example. The actual
exception type may differ in each REST catalog implementation.
If we want to distinguish between tables and views, we should use
`self.table_exists` instead of relying on an exception-driven approach, as Drew
already said.
--
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]
