soarez commented on code in PR #14794:
URL: https://github.com/apache/kafka/pull/14794#discussion_r1398369411
##########
core/src/test/java/kafka/server/AssignmentsManagerTest.java:
##########
@@ -72,6 +75,29 @@ void tearDown() throws InterruptedException {
manager.close();
}
+ AssignReplicasToDirsRequestData normalize(AssignReplicasToDirsRequestData
request) {
Review Comment:
@divijvaidya that sounds like a good idea to me. This list sorting aspect
probably applies to other schemas too. Maybe we could extend the message
generator framework to allow specifying whether the order matters in
`FieldType.ArrayType`. At the moment I'm focussed on making as much progress on
JBOD as possible for 3.7, but I'm happy to come back and tackle this later if
no else picks it up in the meantime.
@ijuma because this is a test, so reasons not to copy don't really take
place such as extra time, extra memory footprint; but the reasons to copy do:
minimizes side effects from the caller perspective, especially as this method
is exclusively used as a utility to `assertRequestEquals` — it would be
surprising to find that a method that verifies equality is also modifying its
arguments.
##########
core/src/test/java/kafka/server/AssignmentsManagerTest.java:
##########
@@ -72,6 +75,29 @@ void tearDown() throws InterruptedException {
manager.close();
}
+ AssignReplicasToDirsRequestData normalize(AssignReplicasToDirsRequestData
request) {
Review Comment:
@divijvaidya that sounds like a good idea to me. This list sorting aspect
probably applies to other schemas too. Maybe we could extend the message
generator framework to allow specifying whether the order matters in
`FieldType.ArrayType`. At the moment I'm focussed on making as much progress on
JBOD as possible for 3.7, but I'm happy to come back and tackle this later if
no else picks it up in the meantime.
@ijuma because this is a test, so reasons not to copy don't really take
place such as extra time, extra memory footprint; but the reasons to copy do:
minimizes side effects from the caller perspective, especially as this method
is exclusively used as a utility to `assertRequestEquals` — it would be
surprising to find that a method that verifies equality is also modifying its
arguments.
--
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-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
