ijuma commented on code in PR #18845: URL: https://github.com/apache/kafka/pull/18845#discussion_r1949581070
########## core/src/main/scala/kafka/cluster/Partition.scala: ########## @@ -1813,7 +1813,7 @@ class Partition(val topicPartition: TopicPartition, private def submitAlterPartition(proposedIsrState: PendingPartitionChange): CompletableFuture[LeaderAndIsr] = { debug(s"Submitting ISR state change $proposedIsrState") val future = alterIsrManager.submit( - new TopicIdPartition(topicId.getOrElse(Uuid.ZERO_UUID), topicPartition), + new org.apache.kafka.server.common.TopicIdPartition(topicId.getOrElse(throw new IllegalStateException("Topic id not set")), topicPartition.partition), Review Comment: > I think this makes sense. I'm not sure exactly though what the error handling is here when an error is thrown. It seems like the futures here are not always checked. (Also wondering if we want to include more information about the partition in the error message) My assumption is that the topic id is actually always set when this method is called, so the error thrown is just a way to make sure we are not missing something. Once we refactor things, this will be more obvious, but it would be nice to avoid blocking 4.0 on that. -- 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