Delete Kafka consumer group on Hermes subscription deletion #1921
Conversation
MarcinBobinski
requested review from
moscicky,
Diewa and
szczygiel-m
and removed request for
moscicky
| @@ -80,4 +86,31 @@ public void createConsumerGroup(Topic topic, Subscription subscription) { | |||
| e); | |||
| } | |||
| } | |||
|
|
|||
| @Override | |||
| public void deleteConsumerGroup(Topic topic, Subscription subscription) { | |||
There was a problem hiding this comment.
Is there a need to pass the whole Topic and Subscription as a parameters?
| @@ -151,6 +157,68 @@ class KafkaConsumerGroupManagerSpec extends Specification { | |||
| kafkaContainer.dockerClient.unpauseContainerCmd(containerId).exec() | |||
| } | |||
|
|
|||
| def "Should delete specified consumer group and retain others"() { | |||
There was a problem hiding this comment.
Nice. Can we also add some integration test for that mechanism? I mean the one which is creating topic and subscription (with validation of creation of consumer-group), deleting it and then checking whether consumer group was deleted too. What do You think about it?
There was a problem hiding this comment.
sure, good idea
| @Override | ||
| public void deleteConsumerGroup(Topic topic, Subscription subscription) { | ||
| logger.info( | ||
| "Deleting consumer group for subscription {}, cluster: {}", |
There was a problem hiding this comment.
I know that this was already done in createConsumerGroup but I would suggest we use DC instead of clusterName for logging purposes. It makes debugging easier
| private final MultiDatacenterRepositoryCommandExecutor multiDcExecutor; | ||
| private final SubscriptionOwnerCache subscriptionOwnerCache; | ||
| private final SubscriptionRepository subscriptionRepository; | ||
|
|
||
| public SubscriptionRemover( | ||
| Auditor auditor, | ||
| MultiDCAwareService multiDCAwareService, |
There was a problem hiding this comment.
not really scope of this PR but MultiDCAwareService vs MultiDatacenterRepositoryCommandExecutor is a mess. What is the difference between these two? MultiDCAwareService even uses MultiDatacenterRepositoryCommandExecutor so using both of them in the same class is confusing
…N-3271-KafkaConumerGroupDeletion
No description provided.