MINOR: Provide default no-op implementations for addReconfigurable and removeReconfigurable in AbstractKafkaConfig#22342
Conversation
…d removeReconfigurable in AbstractKafkaConfig
github-actions
Bot
added
core
| * | ||
| * @param reconfigurable the component to register for configuration updates | ||
| */ | ||
| public abstract void addReconfigurable(Reconfigurable reconfigurable); |
There was a problem hiding this comment.
There are currently 5 dependants for this
NodeToControllerChannelManagerImpl, SocketServer, TransactionMarkerChannelManager, NetworkUtils and BrokerBlockingSender. These are fine as they have already overriden those methds. But any new classes which are not overriding would not receive any dynamic updates, (as there will not be any compile errors, )where they are supposed to ?
There was a problem hiding this comment.
Good catch. To clarify, the 5 classes you listed are callers of addReconfigurable, not subclasses of AbstractKafkaConfig, so this change doesn't affect them.
Today, the only production subclass is kafka.server.KafkaConfig, and it still overrides both methods by delegating to dynamicConfig.
My understanding is that these methods are transitional for the KAFKA-15853 migration: AbstractKafkaConfig is intended to become the future KafkaConfig, so the current dynamicConfig-backed implementation in KafkaConfig should eventually move here as part of that migration.
Please let me know if my understanding is incorrect.
There was a problem hiding this comment.
Ah right, thanks for the clarification. That migration should cover it up. If DynamicBrokerConfig is moved to server, may be thse defaults go away.
…d removeReconfigurable in AbstractKafkaConfig (apache#22342) Ref: apache#22302 (comment). Convert `addReconfigurable` and `removeReconfigurable` from abstract to default no-op methods so tests no longer need to supply dummy overrides. Reviewers: Ken Huang <s7133700@gmail.com>, Murali Basani <muralidhar.basani@aiven.io>, Chia-Ping Tsai <chia7712@gmail.com>
4 participants
Ref: #22302 (comment).
Convert
addReconfigurableandremoveReconfigurablefrom abstract todefault no-op methods so tests no longer need to supply dummy overrides.
Reviewers: Ken Huang s7133700@gmail.com, Murali Basani
muralidhar.basani@aiven.io, Chia-Ping Tsai chia7712@gmail.com