Hello, I have some concerns about the naming: 1. continue() is not feasible because continue is a reserved keyword. Therefore, I suggest using the following names: * continueProcessing * failProcessing * retryProcessing 2. ProductionExceptionHandlerResponse() is already used as the name of an enum in ProductionExceptionHandler interface. This would require adding package names to distinguish it. To avoid confusion, I propose renaming it to: * ProductionExceptionResponse * DeserializationExceptionResponse * ProcessingExceptionResponse 3. I also propose nesting the class into the interface, as it is done for enums.
cheers ! Sébastien ________________________________ De : Matthias J. Sax <mj...@apache.org> Envoyé : samedi 7 décembre 2024 00:39 À : dev@kafka.apache.org <dev@kafka.apache.org> Objet : [EXT] Re: [VOTE] KIP-1034: Dead letter queue in Kafka Streams Warning External sender Do not click on any links or open any attachments unless you trust the sender and know the content is safe. Thanks for raising this issue. Passing in a mutable list as parameter, does not sound to be best practice. Also, some handlers support RETRY option (and there is a Jira ticket to maybe add RETRY to the other handlers, too), for which it does not make sense to write anything into the DLQ. Thus, I would recommend to deprecate the existing handler methods, and add new ones with new name and different return type. The return type would be a class, with static builder methods for each option, allowing both "fail" and "continue" to accept a list of records for the DLQ. Eg public class ProductionExceptionHandlerResponse { private ProductionExceptionHandlerResponse() { }; public static ProductionExceptionHandlerResponse continue(); public static ProductionExceptionHandlerResponse continue(List); public static ProductionExceptionHandlerResponse fail(); public static ProductionExceptionHandlerResponse fail(List); public static ProductionExceptionHandlerResponse retry(); // no overload which would accept a List for "retry" } It might be good to have overloads for "continue/fail" with zero parameters to avoid the need to pass an empty list, what would be annoying boilerplate code for the user. We could rename the methods from `handle` to `handleError` (and `handleSerializationError` for the second handler method of `ProductionExceptionHandler#handleSerializationException`). Thoughts? -Matthias This email was screened for spam and malicious content but exercise caution anyway. On 12/5/24 7:23 AM, Lucas Brutschy wrote: > Hi Sebastien, > > I believe the new proposal has a similar problem (since fields in > interfaces are always static). Would you consider adding an overload > of `handle` instead that passes a collection of DLQ records that can > be mutated? That wouldn't be as clean as returning it in the return > value, but would be correct and easy to implement. Needs a good > Javadoc comment though! > > Cheers, > Lucas > > On Wed, Dec 4, 2024 at 10:26 PM Sebastien Viale > <sebastien.vi...@michelin.com> wrote: >> >> Hi all, >> >> During the implementation of the KIP, we realized that the proposal adds >> some mutable fields to enums: >> >> - ProductionExceptionHandlerResponse from ProductionExceptionHandler >> - DeserializationHandlerResponse from DeserializationExceptionHandler >> - ProcessingHandlerResponse from ProcessingExceptionHandler >> >> This is considered an anti-pattern. >> >> To ensure a cleaner design, we decided to remove mutable fields and methods >> from the enums and move them into their respective classes: >> >> - ProductionExceptionHandler >> - DeserializationExceptionHandler >> - ProcessingExceptionHandler >> >> We have updated the KIP accordingly and are now calling for a new vote to >> maintain transparency. >> >> Cheers, >> Loïc, Damien, and Sébastien >> >> >> ________________________________ >> De : Damien Gasparina <damien@gasparina.cloud> >> Envoyé : vendredi 27 septembre 2024 09:06 >> À : dev@kafka.apache.org <dev@kafka.apache.org> >> Objet : [EXT] Re: [VOTE] KIP-1034: Dead letter queue in Kafka Streams >> >> Warning External sender Do not click on any links or open any attachments >> unless you trust the sender and know the content is safe. >> >> Thanks Bill, Bruno, Sophie, I’ll conclude the vote with your 3 binding votes. >> >> Thank you everyone! >> >> Damien >> >> This email was screened for spam and malicious content but exercise caution >> anyway. >> >> >> >> On Thu, 26 Sept 2024 at 15:21, Bill Bejeck <bbej...@gmail.com> wrote: >>> >>> Thanks for the KIP, this will be a great addition. >>> >>> +1(binding) >>> >>> Regards, >>> Bill >>> >>> On Thu, Sep 26, 2024 at 9:19 AM Bruno Cadonna <cado...@apache.org> wrote: >>> >>>> Thanks Loïc, Sebastien, and Damien, >>>> >>>> +1 (binding) >>>> >>>> Best, >>>> Bruno >>>> >>>> On 9/26/24 3:15 AM, Sophie Blee-Goldman wrote: >>>>> +1 (binding) >>>>> >>>>> thanks for the KIP guys! >>>>> >>>>> On Mon, Sep 23, 2024 at 3:38 AM Sebastien Viale < >>>>> sebastien.vi...@michelin.com> wrote: >>>>> >>>>>> Hi everyone, >>>>>> >>>>>> Just a quick reminder that the vote for KIP-1034 is still open. >>>>>> Thank you all for your participation! >>>>>> >>>>>> Best regards, >>>>>> Damien Sebastien and Loic >>>>>> >>>>>> >>>>>> ________________________________ >>>>>> De : Sebastien Viale <sebastien.vi...@michelin.com> >>>>>> Envoyé : mercredi 11 septembre 2024 09:26 >>>>>> À : dev <dev@kafka.apache.org> >>>>>> Objet : Marketing: [VOTE] KIP-1034: Dead letter queue in Kafka Streams >>>>>> >>>>>> Hi all, >>>>>> >>>>>> We would like to start a vote for KIP-1034: Dead letter queue in Kafka >>>>>> Streams< >>>>>> >>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-1034%3A+Dead+letter+queue+in+Kafka+Streams<https://cwiki.apache.org/confluence/display/KAFKA/KIP-1034%3A+Dead+letter+queue+in+Kafka+Streams><https://cwiki.apache.org/confluence/display/KAFKA/KIP-1034%3A+Dead+letter+queue+in+Kafka+Streams<https://cwiki.apache.org/confluence/display/KAFKA/KIP-1034%3A+Dead+letter+queue+in+Kafka+Streams>> >>>>>> < >>>>>> >>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-1034%3A+Dead+letter+queue+in+Kafka+Streams<https://cwiki.apache.org/confluence/display/KAFKA/KIP-1034%3A+Dead+letter+queue+in+Kafka+Streams><https://cwiki.apache.org/confluence/display/KAFKA/KIP-1034%3A+Dead+letter+queue+in+Kafka+Streams<https://cwiki.apache.org/confluence/display/KAFKA/KIP-1034%3A+Dead+letter+queue+in+Kafka+Streams>> >>>>>>>> >>>>>> >>>>>> The KIP is available on >>>>>> >>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-1034%3A+Dead+letter+queue+in+Kafka+Streams<https://cwiki.apache.org/confluence/display/KAFKA/KIP-1034%3A+Dead+letter+queue+in+Kafka+Streams><https://cwiki.apache.org/confluence/display/KAFKA/KIP-1034%3A+Dead+letter+queue+in+Kafka+Streams<https://cwiki.apache.org/confluence/display/KAFKA/KIP-1034%3A+Dead+letter+queue+in+Kafka+Streams>> >>>>>> < >>>>>> >>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-1034%3A+Dead+letter+queue+in+Kafka+Streams<https://cwiki.apache.org/confluence/display/KAFKA/KIP-1034%3A+Dead+letter+queue+in+Kafka+Streams><https://cwiki.apache.org/confluence/display/KAFKA/KIP-1034%3A+Dead+letter+queue+in+Kafka+Streams<https://cwiki.apache.org/confluence/display/KAFKA/KIP-1034%3A+Dead+letter+queue+in+Kafka+Streams>> >>>>>>> >>>>>> >>>>>> If you have any suggestions or feedback, feel free to participate to the >>>>>> discussion thread: >>>>>> https://lists.apache.org/thread/1nhhsrogmmv15o7mk9nj4kvkb5k2bx9s<https://lists.apache.org/thread/1nhhsrogmmv15o7mk9nj4kvkb5k2bx9s><https://lists.apache.org/thread/1nhhsrogmmv15o7mk9nj4kvkb5k2bx9s<https://lists.apache.org/thread/1nhhsrogmmv15o7mk9nj4kvkb5k2bx9s>>< >>>>>> https://lists.apache.org/thread/1nhhsrogmmv15o7mk9nj4kvkb5k2bx9s<https://lists.apache.org/thread/1nhhsrogmmv15o7mk9nj4kvkb5k2bx9s><https://lists.apache.org/thread/1nhhsrogmmv15o7mk9nj4kvkb5k2bx9s<https://lists.apache.org/thread/1nhhsrogmmv15o7mk9nj4kvkb5k2bx9s>>> >>>>>> >>>>>> Best regards, >>>>>> >>>>>> Damien Sebastien and Loic >>>>>> >>>>>> >>>>>> This email was screened for spam and malicious content but exercise >>>>>> caution anyway. >>>>>> >>>>>> >>>>>> >>>>> >>>> >>>>