(trunk) KAFKA-5232 Fix Log.parseTopicPartitionName to take into account dot character in topic names of deleted topics #3050
Conversation
| @@ -1640,6 +1640,18 @@ class LogTest { | |||
| assertEquals(partition.toInt, topicPartition.partition) | |||
| } | |||
|
|
|||
| // see https://issues.apache.org/jira/browse/KAFKA-5232 | |||
There was a problem hiding this comment.
This is simple enough that we can write a short description here instead of referencing the JIRA.
| @@ -1640,6 +1640,18 @@ class LogTest { | |||
| assertEquals(partition.toInt, topicPartition.partition) | |||
| } | |||
|
|
|||
| // see https://issues.apache.org/jira/browse/KAFKA-5232 | |||
| @Test | |||
| def testParseTopicPartitionNameForDeletedTopic() { | |||
There was a problem hiding this comment.
testParseTopicPartitionNameWithPeriodForDeletedTopic?
| val topicPartition = Log.parseTopicPartitionName(dir) | ||
| assertEquals("Unexpected topic name parsed", topic, topicPartition.topic) | ||
| assertEquals("Unexpected partition number parsed", partition.toInt, topicPartition.partition) | ||
| } |
There was a problem hiding this comment.
While we're doing this, it would be better to flesh out tests for the deleted case. Namely variants of testParseTopicPartitionNameForMissingSeparator, testParseTopicPartitionNameForMissingTopic and testParseTopicPartitionNameForMissingPartition with the -delete suffix.
|
Refer to this link for build results (access rights to CI server needed): |
…aking into account the potential presence of '.' character in topic names while looking for topics that are marked for deletion
|
@ijuma, thanks for reviewing. I have updated this PR to take into account your review comments. I updated those existing test methods to include checks for the |
|
Refer to this link for build results (access rights to CI server needed): |
|
@jaikiran, I did a PR to your branch with additional improvements: https://github.com/jaikiran/kafka/pull/1 Please merge if they look good to you. |
|
Refer to this link for build results (access rights to CI server needed): |
Various improvements to `parseTopicPartitionName` and tests
|
Done. Those changes looked good to me. |
|
Refer to this link for build results (access rights to CI server needed): |
|
Refer to this link for build results (access rights to CI server needed): |
The commit here contains a fix and a test case for the issue reported in https://issues.apache.org/jira/browse/KAFKA-5232. This PR is meant for
trunkbranch and a corresponding PR for0.10.2branch has been raised here #3043