krvikash commented on code in PR #6500:
URL: https://github.com/apache/iceberg/pull/6500#discussion_r1058723448
##########
aws/src/main/java/org/apache/iceberg/aws/glue/GlueTableOperations.java:
##########
@@ -184,9 +185,7 @@ protected void doCommit(TableMetadata base, TableMetadata
metadata) {
if (persistFailure instanceof AwsServiceException) {
int statusCode = ((AwsServiceException) persistFailure).statusCode();
- if (statusCode >= 500 && statusCode < 600) {
- commitStatus = CommitStatus.FAILURE;
- } else {
+ if (statusCode < 500 && statusCode >= 600) {
Review Comment:
Thanks, @amogh-jahagirdar for the review.
I am not changing the meaning of the code as compared to earlier. It will
work the same way as earlier. Since `commitStatus` is already set to
`CommitStatus.FAILURE` at the beginning of `doCommit` method then we don't need
to set it again when `AwsServiceException` `statusCode` >= 500 and < 600.
> A retry commit is performed which fails with a status code >= 500 and <
600 then commit status then falls through to the checkCommitStatus again
No that's not true, `checkCommitStatus` will be only called for
non-AwsServiceException. Any `AwsServiceException` which does not fall under >=
500 and < 600 will have the exception `persistFailure` and others will have to
go through `switch` statement and throw the exception according to
`commitStatus` value (in this case it will always be `CommitStatus.FAILURE`).
For simplicity and readability, the `statusCode` check can be changed to,
```java
if (!(statusCode >= 500 && statusCode < 600)) {
throw persistFailure;
}
```
My whole point was to remove redundant assigning to `commitStatus` since it
was already set at the beginning of this method call.
--
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]
