unknowntpo commented on code in PR #22409:
URL: https://github.com/apache/kafka/pull/22409#discussion_r3487782594


##########
storage/src/main/java/org/apache/kafka/storage/internals/log/LogCleaner.java:
##########
@@ -95,7 +95,7 @@
  *    tombstone deletion.</li>
  * </ol>
  */
-public class LogCleaner implements BrokerReconfigurable {
+public class LogCleaner implements BrokerReconfigurable<AbstractConfig> {

Review Comment:
   Thanks Chia, that makes sense. I agree the `DynamicConfigurable` + generic 
adapter approach was more abstraction than we need here.
   
   I updated the PR to follow the existing named-adapter style: 
`BrokerReconfigurable` now lives in server and uses `AbstractKafkaConfig` 
directly, while `LogCleaner` keeps its own `CleanerConfig`-based methods (so 
storage no longer depends on the broker reconfig interface). The server side 
now has a simple `DynamicLogCleanerConfig` adapter for the `AbstractKafkaConfig 
-> CleanerConfig` conversion.
   
   I also checked there is no leftover `DynamicConfigurable`, generic adapter, 
generic `BrokerReconfigurable<T>`, or Scala `_ >: KafkaConfig`. PTAL.



-- 
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]

Reply via email to