Copilot commented on code in PR #5138:
URL: https://github.com/apache/polaris/pull/5138#discussion_r3643925258
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java:
##########
@@ -1531,7 +1532,13 @@ public void commitTransaction(CommitTransactionRequest
commitTransactionRequest)
}
// Apply updates to builder
- singleUpdate.applyTo(metadataBuilder);
+ try {
+ singleUpdate.applyTo(metadataBuilder);
+ } catch (RetryableValidationException e) {
+ // Surface as a retryable 409, matching
CatalogHandlerUtils.commit.
+ throw new CommitFailedException(
+ e, "Validation failed, please retry: %s", e.getMessage());
+ }
Review Comment:
This change alters user-visible behavior (RetryableValidationException now
maps to a retryable 409 via CommitFailedException rather than a fatal 400).
Please consider whether this should be called out in CHANGELOG.md and/or
user-facing REST error code documentation.
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/CatalogHandlerUtils.java:
##########
@@ -513,7 +514,14 @@ public TableMetadata commit(TableOperations ops,
UpdateTableRequest request) {
}
TableMetadata.Builder newMetadataBuilder =
TableMetadata.buildFrom(newBase);
- request.updates().forEach((update) ->
update.applyTo(newMetadataBuilder));
+ try {
+ request.updates().forEach((update) ->
update.applyTo(newMetadataBuilder));
+ } catch (RetryableValidationException e) {
+ // Surface as a retryable 409, matching Iceberg
CatalogHandlers.commit.
+ throw new ValidationFailureException(
+ new CommitFailedException(
+ e, "Validation failed, please retry: %s",
e.getMessage()));
Review Comment:
In CatalogHandlerUtils.commit(), throwing ValidationFailureException here
bypasses the Tasks.foreach(...).onlyRetryOn(CommitFailedException.class) retry
loop, so RetryableValidationException will not be retried server-side. If the
intent is to match Iceberg CatalogHandlers.commit (and treat this as
retryable), rethrow CommitFailedException directly so the existing
retry/backoff logic can run.
--
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]
