ableegoldman commented on a change in pull request #8497:
URL: https://github.com/apache/kafka/pull/8497#discussion_r411858555
##########
File path:
streams/src/test/java/org/apache/kafka/streams/processor/internals/StreamsPartitionAssignorTest.java
##########
@@ -1610,79 +1610,6 @@ public void
shouldReturnInterleavedAssignmentWithUnrevokedPartitionsRemovedWhenN
)));
}
- @Test
- public void
shouldReturnNormalAssignmentForOldAndFutureInstancesDuringVersionProbing() {
Review comment:
I don't mean to totally cop out on this, but I think we should do this
in a followup PR. I'll make a ticket and assign it to myself for later so I
can't escape, but I don't even think it's worth marking it `@Ignore` for now.
Tbh we should have removed it a while ago, rather than changing it over time
to become its useless self today. It's a long history, and I'm mostly
responsible, but just looking ahead the question now is: what do we even want
to validate? The task assignor has no knowledge of version probing, and the
partition assignor is not responsible for the task assignment (whereas it used
to be with version probing, hence this test). What we should do is validate the
inputs are being assembled sensibly during version probing.
Anyways this will be really difficult to do just based on the final
partition assignment, and even harder to distinguish a real failure from an
unrelated one. So I'd propose to kick this into the future, when we embed the
actual assignor class in the configs instead of this flag, and then pass in a
`VersionProbingClientStatesValidatingAssignor` or whatever...SG?
----------------------------------------------------------------
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:
us...@infra.apache.org
