You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2022/06/24 05:13:33 UTC
[GitHub] [kafka] chia7712 opened a new pull request, #12339: MINOR: set UNKNOWN_MEMBER_ID explicitly when deleting group instance id
chia7712 opened a new pull request, #12339:
URL: https://github.com/apache/kafka/pull/12339
This is not a bug fix, but it makes code more readable. We expect the member id to be undefined(`UNKNOWN_MEMBER_ID`) when we are removing group instance id. Hence, it would be better to set `UNKNOWN_MEMBER_ID` explicitly.
### Committer Checklist (excluded from commit message)
- [ ] Verify design and implementation
- [ ] Verify test coverage and CI build status
- [ ] Verify documentation (including upgrade notes)
--
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.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] chia7712 closed pull request #12339: MINOR: set UNKNOWN_MEMBER_ID explicitly when deleting group instance id
Posted by GitBox <gi...@apache.org>.
chia7712 closed pull request #12339: MINOR: set UNKNOWN_MEMBER_ID explicitly when deleting group instance id
URL: https://github.com/apache/kafka/pull/12339
--
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.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] dajac commented on a diff in pull request #12339: MINOR: set UNKNOWN_MEMBER_ID explicitly when deleting group instance id
Posted by GitBox <gi...@apache.org>.
dajac commented on code in PR #12339:
URL: https://github.com/apache/kafka/pull/12339#discussion_r906064565
##########
clients/src/main/java/org/apache/kafka/clients/admin/KafkaAdminClient.java:
##########
@@ -3742,7 +3743,8 @@ private List<MemberIdentity> getMembersFromGroup(String groupId, String reason)
MemberIdentity memberIdentity = new MemberIdentity().setReason(reason);
if (member.groupInstanceId().isPresent()) {
- memberIdentity.setGroupInstanceId(member.groupInstanceId().get());
+ memberIdentity.setGroupInstanceId(member.groupInstanceId().get())
+ .setMemberId(JoinGroupRequest.UNKNOWN_MEMBER_ID);
Review Comment:
Is it really better? I am not convinced because `UNKNOWN_MEMBER_ID` is the default value anyway. If we follow this train of thoughts, why don't we add `setGroupInstanceId(null)` to the else branch as well?
--
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.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] chia7712 commented on a diff in pull request #12339: MINOR: set UNKNOWN_MEMBER_ID explicitly when deleting group instance id
Posted by GitBox <gi...@apache.org>.
chia7712 commented on code in PR #12339:
URL: https://github.com/apache/kafka/pull/12339#discussion_r906090123
##########
clients/src/main/java/org/apache/kafka/clients/admin/KafkaAdminClient.java:
##########
@@ -3742,7 +3743,8 @@ private List<MemberIdentity> getMembersFromGroup(String groupId, String reason)
MemberIdentity memberIdentity = new MemberIdentity().setReason(reason);
if (member.groupInstanceId().isPresent()) {
- memberIdentity.setGroupInstanceId(member.groupInstanceId().get());
+ memberIdentity.setGroupInstanceId(member.groupInstanceId().get())
+ .setMemberId(JoinGroupRequest.UNKNOWN_MEMBER_ID);
Review Comment:
@dajac thanks for this feedback. You are right :)
--
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.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org