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: issues-unsubscr...@flink.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Reply via email to