CrynetLogistics commented on a change in pull request #18553:
URL: https://github.com/apache/flink/pull/18553#discussion_r795522964
##########
File path:
flink-connectors/flink-connector-aws-kinesis-data-streams/src/main/java/org/apache/flink/connector/kinesis/sink/KinesisDataStreamsException.java
##########
@@ -39,12 +39,12 @@ public KinesisDataStreamsException(final String message,
final Throwable cause)
public KinesisDataStreamsFailFastException() {
super(
- "Encountered an exception while persisting records, not
retrying due to {failOnError} being set.");
+ "Encountered an exception while persisting records, not
retrying due to either: {failOnError} being set or the exception should not be
retried.");
}
public KinesisDataStreamsFailFastException(final Throwable cause) {
super(
- "Encountered an exception while persisting records, not
retrying due to {failOnError} being set.",
+ "Encountered an exception while persisting records, not
retrying due to either: {failOnError} being set or the exception should not be
retried.",
Review comment:
I'm currently wrapping any exception that I think is not retryable with
the `KinesisDataStreamsException`, if we encounter it in the KDSSink.
I hear you, maybe it will be better to throw any `Error`s and
`RuntimeException`s directly without wrapping. This way these non-checked
exceptions would be thrown without a KDSException wrapper confusing things -
but obvs they must still be accepted by the consumer because even non-checked
exceptions aren't fatal until they are consumed by the mailbox.
Also agree we can split failOnError and not retrying exceptions.
--
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]
