Skip to content

Clean up Consumer/ConsumerProducer#421

Draft
philipp94831 wants to merge 15 commits intomasterfrom
feature/clean-up-consumer-producer
Draft

Clean up Consumer/ConsumerProducer#421
philipp94831 wants to merge 15 commits intomasterfrom
feature/clean-up-consumer-producer

Conversation

@philipp94831
Copy link
Member

@philipp94831 philipp94831 commented Feb 26, 2026

Follow up to #385 and #387

@philipp94831 philipp94831 self-assigned this Feb 26, 2026
log.debug("Closing consumer runnable");
this.consumerRunnable.close();
} catch (final RuntimeException e) {
//TODO why catch?
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@raphala why are you catching here?

log.debug("Closing producer");
this.producer.close();
} catch (final RuntimeException e) {
//TODO why catch?
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@raphala why are you catching here?

} catch (final WakeupException exception) {
log.info("Consumer poll loop waking up for shutdown", exception);
} catch (final RuntimeException exception) {
//TODO why catch?
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@raphala why are you catching here?

.config(this.config)
.build();
log.debug("Calling start hook");
this.executionOptions.onStart(runningConsumer);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@raphala @jkbe Do we even need that? I don't think so. For streams it exists to give the use access to the KafkaStreams instance for building application servers. For the consumer and consumer-producer, the user already has full control because the user implements the complete runnable. I would remove it tbh

@sonarqubecloud
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants