adutra commented on code in PR #3734:
URL: https://github.com/apache/polaris/pull/3734#discussion_r3274217508
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java:
##########
@@ -631,7 +655,9 @@ private LoadTableResponse registerTableWithOverwrite(
Table table = icebergCatalog.registerTable(identifier,
request.metadataLocation(), true);
if (table instanceof BaseTable baseTable) {
TableMetadata metadata = baseTable.operations().current();
- return LoadTableResponse.builder().withTableMetadata(metadata).build();
+ return buildLoadTableResponseWithDelegationCredentials(
+ identifier, metadata, delegationModes, actionsRequested,
refreshCredentialsEndpoint)
+ .build();
}
throw new IllegalStateException("Cannot wrap catalog that does not
produce BaseTable");
Review Comment:
I agree that the current message is cryptic, but your suggestion leaks
internal implementation details to clients, so I'm a bit reluctant to follow
it.
OTOH, this `throw` statement is imho never reached in practice.
How about:
```java
throw new IllegalStateException(
"Cannot register table %s: unknown table
format".formatted(identifier));
```
?
--
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]
