Abacn commented on code in PR #37166:
URL: https://github.com/apache/beam/pull/37166#discussion_r2647305558
##########
sdks/java/io/elasticsearch/src/main/java/org/apache/beam/sdk/io/elasticsearch/ElasticsearchIO.java:
##########
@@ -2811,13 +2811,14 @@ protected void addAndMaybeFlush(Document doc,
ProcessContext context)
}
private boolean isRetryableClientException(Throwable t) {
- // RestClient#performRequest only throws wrapped IOException so we
must inspect the
+ // RestClient#performRequest mainly throws wrapped IOException so we
must inspect the
// exception cause to determine if the exception is likely transient
i.e. retryable or
- // not.
+ // not. One exception is the ResponseException which is thrown when
attempting to parse the
+ // response. This exception is not wrapped.
// Retry for 500-range response code except for 501.
- if (t.getCause() instanceof ResponseException) {
- ResponseException ex = (ResponseException) t.getCause();
+ if (t instanceof ResponseException) {
Review Comment:
Thanks. This makes sense to me. Wasn't sure why it assumed `t.getCause()`
may be `ResponseException` instead of the Exception itself. It may be a bug or
behavior of older ElasticSearch versions and get missed as ElasticSearch
evolving. As a precaution shall we support both scenario?
--
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]
