IgnacioAcunaF commented on pull request #10858:
URL: https://github.com/apache/kafka/pull/10858#issuecomment-862046292


   @dajac Thanks again for comments and support! :)
   Updated the PR. I think I improved the unit test. Revert some changes and 
keep the unit test isolated from the others, but trying to keep the same logic. 
 Tried to step on all the possible scenarios:
   
   - a) Assigned topic partition with no offset data at all  -> Response: None
   - b) Assigned topic partition with a valid OffsetAndMetadata -> Response: 
Long
   - c) Assigned topic partition with an invalidad OffsetAndMetadata (null 
case) -> Response: None
   - d) Unassigned topic partition with no offset data at all -> Response: 
nothing, not on the resulting List
   - e) Unassigned topic partition with a valid OffsetAndMetadata -> Response: 
Long
   - f) Unassigned topic partition with an invalidad OffsetAndMetadata (null 
case) -> Response: None
   
   Also modified a little bit the getPartitionOffsets to adapt it to being able 
to call directly to collectConsumerAssignment, as suggested.
   
   **PD:** As I posted on the comments earlier I encountered that the sibling 
test, _testAdminRequestsForDescribeOffsets_, lacks the validation for assigned 
topic partitions (it is currently only doing the validation against unassigned 
topic partitions, basically because there is not an topic's assigment to the 
testing consumer group at its initialization). Solved at the new test 
_testAdminRequestsForDescribeNegativeOffsets_, and I think that I could 
complement the former one. 
   
   What do you think? Is it worth to open a new PR to approach that separatly?


-- 
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


Reply via email to