hachikuji commented on pull request #10005:
URL: https://github.com/apache/kafka/pull/10005#issuecomment-772130047
@rondagostino Thanks for the updates. I am wondering if we can pull out the
changes in `KafkaApis` since most of them are unrelated to the
`ConfigRepository` refactor. In general, I think we need a better approach for
managing the dependency requirements for the zk/self-managed implementations.
One idea here is try to encapsulate the zk-specific dependencies into a
separate object. For example:
```scala
trait MetadataSupport
class ZkSupport(
adminManager: ZkAdminManager,
zkClient: KafkaZkClient,
...
) extends MetadataSupport
```
Then we can pass through `MetadataSupport` and get rid of the optional
types. We can then have helpers like the following:
```scala
def requireZk(): ZkSupport = {
support match {
case zkSupport: ZkSupport => zkSupport
case _ => throw new IllegalStateException
}
}
```
The advantage is that we can make the dependencies that are available when
zk support is enabled explicit. We don't have to deal with the permutations
that are possible when we have 5 different `Option` classes.
We can then start making this trait more useful by adding methods. For
example, instead of reaching into the `KafkaController` to get the broker
epoch, we can add a `brokerEpoch` method. Eventually we might get to a point
where we don't need so many special cases.
The second thing I think we can do better is how we handle the acceptable
scope of the APIs when using zk or the self-managed quorum. The patch does this
by adding separate checks in each of the individual handlers. I filed
https://issues.apache.org/jira/browse/KAFKA-12278 to try and handle this
problem in a more general way, and so that we can also keep ApiVersions
consistent. I would rather address this problem this problem in that JIRA if it
is ok with you.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
