[GitHub] [kafka] hachikuji commented on pull request #10005: MINOR: Add ConfigRepository, use in Partition and KafkaApis

[GitBox <gi...@apache.org>]

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:
users@infra.apache.org