andr-mueller commented on PR #29618: URL: https://github.com/apache/camel/pull/29618#issuecomment-6795305939
Thanks @davsclaus and @gnodet for the thorough review. I pushed an update that addresses the feedback: 1. Lifecycle wording. Reworded the Javadoc on PollExceptionStrategyFactory and KafkaComponent#setPollExceptionStrategyFactory, and the option description, to say the factory is invoked each time a fetch thread creates its Kafka consumer (on start and on every reconnect), that it may be called concurrently and must be thread-safe, and that a stateful strategy does not retain state across reconnects. Renamed the test accordingly and regenerated the metadata. Also changed the description to read "Unlike the pollExceptionStrategy option". 2. Docs. Added a paragraph to kafka-component.adoc on pollExceptionStrategyFactory and when to use it (consumersCount > 1, or to wrap a built-in strategy via DefaultPollExceptionStrategyFactory), and regenerated the catalog copy. 3. Static import. KafkaErrorStrategiesTest now uses a static import for ArgumentMatchers.any. Optional items, also done: - Dropped DefaultPollExceptionStrategyFactory#createBuiltIn. A subclass overrides create() and calls super.create(...) to decorate the built-in strategy, which keeps the API smaller. - The factory-precedence test now verifies the fetcher, endpoint, and consumer are passed through. - Added @FunctionalInterface and promoted the default factory to a constant in KafkaErrorStrategies. On the <p/> and <tt> nits: I kept them, per @davsclaus, since they are used throughout Camel and match the surrounding code. -- 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]
