AndrewJSchofield commented on code in PR #16106:
URL: https://github.com/apache/kafka/pull/16106#discussion_r1617352947
##########
clients/src/main/resources/common/message/DescribeQuorumRequest.json:
##########
@@ -19,7 +19,7 @@
"listeners": ["broker", "controller"],
"name": "DescribeQuorumRequest",
// Version 1 adds additional fields in the response. The request is
unchanged (KIP-836).
- "validVersions": "0-1",
+ "validVersions": "0-2",
Review Comment:
A comment for v2 would be useful. I know the request is unchanged, but v2
does have a meaning still.
##########
clients/src/main/java/org/apache/kafka/clients/admin/QuorumInfo.java:
##########
@@ -122,6 +135,13 @@ public int replicaId() {
return replicaId;
}
+ /**
+ * Return the directory id of the replica if configured.
+ */
+ public Optional<Uuid> replicaDirectoryId() {
+ return replicaDirectoryId;
Review Comment:
I think there might be a small problem related to the deserialization of
this. If a v0/v1 DescribeQuorumResponse is received, the value for the
ReplicaDirectoryId is Uuid.ZERO_UUID. This is not null. So, I think you'll not
get `Optional.empty()`, but rather `Optional.of(Uuid.ZERO_UUID)`.
##########
clients/src/main/resources/common/message/FetchResponse.json:
##########
@@ -47,7 +47,7 @@
// Version 15 is the same as version 14 (KIP-903).
//
// Version 16 adds the 'NodeEndpoints' field (KIP-951).
- "validVersions": "0-16",
+ "validVersions": "0-17",
Review Comment:
Probably ought to have a comment for version 17 too.
##########
clients/src/main/resources/common/message/DescribeQuorumResponse.json:
##########
@@ -40,10 +44,27 @@
{ "name": "CurrentVoters", "type": "[]ReplicaState", "versions": "0+"
},
{ "name": "Observers", "type": "[]ReplicaState", "versions": "0+" }
]}
- ]}],
+ ]},
+ { "name": "Nodes", "type": "[]Node", "versions": "2+", "fields": [
+ { "name": "NodeId", "type": "int32", "versions": "2+",
+ "mapKey": true, "entityType": "brokerId", "about": "The ID of the
associated node" },
+ { "name": "Listeners", "type": "[]Listener",
+ "about": "The listeners of this controller", "versions": "0+",
"fields": [
Review Comment:
It does seem a bit odd having these fields at version 0+ when the entire
structure is 2+.
##########
clients/src/main/resources/common/message/DescribeQuorumResponse.json:
##########
@@ -18,11 +18,13 @@
"type": "response",
"name": "DescribeQuorumResponse",
// Version 1 adds LastFetchTimeStamp and LastCaughtUpTimestamp in
ReplicaState (KIP-836).
- "validVersions": "0-1",
+ "validVersions": "0-2",
Review Comment:
And another comment for v2 I think.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
