VinceMu commented on a change in pull request #8666:
URL: https://github.com/apache/kafka/pull/8666#discussion_r435218619
##########
File path: core/src/main/scala/kafka/admin/ConsumerGroupCommand.scala
##########
@@ -291,14 +291,21 @@ object ConsumerGroupCommand extends Logging {
}
private def printStates(states: Map[String, GroupState]): Unit = {
- for ((groupId, state) <- states) {
- if (shouldPrintMemberState(groupId, Some(state.state), Some(1))) {
+ val stateProps =
states.filter{case(groupId,state)=>shouldPrintMemberState(groupId,
Some(state.state), Some(1))}
+ .map{case (_,state)=>
val coordinator =
s"${state.coordinator.host}:${state.coordinator.port}
(${state.coordinator.idString})"
- val coordinatorColLen = Math.max(25, coordinator.length)
- print(s"\n%${-coordinatorColLen}s %-25s %-20s %-15s
%s".format("GROUP", "COORDINATOR (ID)", "ASSIGNMENT-STRATEGY", "STATE",
"#MEMBERS"))
- print(s"\n%${-coordinatorColLen}s %-25s %-20s %-15s
%s".format(state.group, coordinator, state.assignmentStrategy, state.state,
state.numMembers))
- println()
+
(state.group,coordinator,state.assignmentStrategy,state.state,state.numMembers)
}
+ val hasAllGroups = opts.options.has(opts.allGroupsOpt)
+ if(stateProps.nonEmpty && hasAllGroups){
+ val headerLengthOffset =
Math.max(25,stateProps.maxBy{_._2.length}._2.length)
+ print(s"\n%${-headerLengthOffset}s %-25s %-20s %-15s
%s".format("GROUP", "COORDINATOR (ID)", "ASSIGNMENT-STRATEGY", "STATE",
"#MEMBERS"))
Review comment:
`Print`s with explicit '\n' are used throughout ConsumerGroupCommand. My
guess as to why this is because `println` uses `System.lineSeparator` which may
not always be '\n' depending on the operating system. Switching to `println`
would also involve modifying the tests which count lines by splitting on `\n'.
Because of these reasons I'm hesitant to switch over to `println`. Thoughts?
The idea of using the coordinator length as an offset to format the length
existed before this PR. The change I've now implemented is instead of
calculating offset length per group, we calculate it only once by taking the
largest possible offset from all groups.
##########
File path: core/src/main/scala/kafka/admin/ConsumerGroupCommand.scala
##########
@@ -291,14 +291,21 @@ object ConsumerGroupCommand extends Logging {
}
private def printStates(states: Map[String, GroupState]): Unit = {
- for ((groupId, state) <- states) {
- if (shouldPrintMemberState(groupId, Some(state.state), Some(1))) {
+ val stateProps =
states.filter{case(groupId,state)=>shouldPrintMemberState(groupId,
Some(state.state), Some(1))}
+ .map{case (_,state)=>
val coordinator =
s"${state.coordinator.host}:${state.coordinator.port}
(${state.coordinator.idString})"
- val coordinatorColLen = Math.max(25, coordinator.length)
- print(s"\n%${-coordinatorColLen}s %-25s %-20s %-15s
%s".format("GROUP", "COORDINATOR (ID)", "ASSIGNMENT-STRATEGY", "STATE",
"#MEMBERS"))
- print(s"\n%${-coordinatorColLen}s %-25s %-20s %-15s
%s".format(state.group, coordinator, state.assignmentStrategy, state.state,
state.numMembers))
- println()
+
(state.group,coordinator,state.assignmentStrategy,state.state,state.numMembers)
}
+ val hasAllGroups = opts.options.has(opts.allGroupsOpt)
+ if(stateProps.nonEmpty && hasAllGroups){
+ val headerLengthOffset =
Math.max(25,stateProps.maxBy{_._2.length}._2.length)
+ print(s"\n%${-headerLengthOffset}s %-25s %-20s %-15s
%s".format("GROUP", "COORDINATOR (ID)", "ASSIGNMENT-STRATEGY", "STATE",
"#MEMBERS"))
Review comment:
`Print`s with explicit '\n' are used throughout ConsumerGroupCommand. My
guess as to why this is because `println` uses `System.lineSeparator` which may
not always be '\n' depending on the operating system. Switching to `println`
would also involve modifying the tests which count lines by splitting on '\n'.
Because of these reasons I'm hesitant to switch over to `println`. Thoughts?
The idea of using the coordinator length as an offset to format the length
existed before this PR. The change I've now implemented is instead of
calculating offset length per group, we calculate it only once by taking the
largest possible offset from all groups.
----------------------------------------------------------------
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:
[email protected]
