[
https://issues.apache.org/jira/browse/KAFKA-5123?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16034575#comment-16034575
]
Balint Molnar commented on KAFKA-5123:
--------------------------------------
How to refactor zkUtils.readData method:
* move all zkException inside ZkUtils
** Class ConsumerGroupCommand:
***
https://github.com/apache/kafka/blob/trunk/core/src/main/scala/kafka/admin/ConsumerGroupCommand.scala#L342
*** It is easy because we can match for None and then call the printerror(…)
** Class ConsumerOffsetChecker:
***
https://github.com/apache/kafka/blob/trunk/core/src/main/scala/kafka/tools/ConsumerOffsetChecker.scala#L172
*** Maybe we can throw a different exception here/ introduce new one which is
not related to zk.
** Class ZkUtils:
***
https://github.com/apache/kafka/blob/trunk/core/src/main/scala/kafka/utils/ZkUtils.scala#L496
*** We can handle this with None
* this method should only return the data without the stat and introduce a new
method with name readDataAndStat which returns the data and the stat too, with
this one we don’t need to call the annoying ._1 every time.
How to refactor zkUtils.readDataMaybeNull method:
* Do not return Some(null) convert Some(null) into None with calling Option()
instead of Some(). Also rename this method to readData. I do not see why we
need a separate method for these two things.
** This method is a little bit tricky because of the Some() no one traits the
Some(null)
** For example:
*** Class ConsumerGroupCommand:
****
https://github.com/apache/kafka/blob/trunk/core/src/main/scala/kafka/admin/ConsumerGroupCommand.scala#L272
**** we are calling a substring on null string
*** Class ZookeeperConsumerConnector:
****
https://github.com/apache/kafka/blob/trunk/core/src/main/scala/kafka/consumer/ZookeeperConsumerConnector.scala#L422
**** we are calling toLong on null string
* This method should only return the data without the stat and introduce a new
method with name readDataAndStat.
How to refactor zkUtils.readDataAndVersionMaybeNull method:
* I think we can remove the MaybeNull from it’s name, otherwise this method
looks ok to me.
[~ijuma] what do you think? If you agree, I will start to implement this.
> Refactor ZkUtils readData* methods
> -----------------------------------
>
> Key: KAFKA-5123
> URL: https://issues.apache.org/jira/browse/KAFKA-5123
> Project: Kafka
> Issue Type: Bug
> Reporter: Balint Molnar
> Assignee: Balint Molnar
> Priority: Minor
>
> Usually only the data value is required but every readData method in the
> ZkUtils returns a Tuple with the data and the stat.
> https://github.com/apache/kafka/pull/2888
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
