Conversation
|
Call for review @guozhangwang @bbejeck @vvcephei |
vvcephei
left a comment
vvcephei
left a comment
There was a problem hiding this comment.
Looks great! Thanks so much for this cleanup.
The only concern is that assignment operator.
| assertThat(topology.processorConnectedStateStores("KSTREAM-JOIN-0000000005"), equalTo(Collections.singleton(topology.stateStores().get(0).name()))); | ||
| assertThat(topology.processorConnectedStateStores("KTABLE-FILTER-0000000003").isEmpty(), is(true)); | ||
| final ProcessorTopology topology | ||
| = builder.internalTopologyBuilder.rewriteTopology(new StreamsConfig(props)).build(); |
There was a problem hiding this comment.
Repeating my comment from #6053 (comment) here... Not sure about the assignment operator at the start of the line.
I won't comment further on this in this PR.
|
|
||
| @Test | ||
| public void shouldCreateSourceAndSinkNodesForRepartitioningTopic() throws NoSuchFieldException, IllegalAccessException { | ||
| public void shouldCreateSourceAndSinkNodesForRepartitioningTopic() throws Exception { |
There was a problem hiding this comment.
Relaxing the throws declaration tightens up the line, but does it make the test more mysterious?
There was a problem hiding this comment.
No (compare my comment on the other PR). If any exception occurs, the test fails anyway.
|
lgtm. @mjsax please feel free to merge after addressing the above comments. |
|
Updates. John, please let me know if I can merge. |
Reviewers: Bill Bejeck <bill@confluent.io>, John Roesler <john@confluent.io>, Guozhang Wang <guozhang@confluent.io>
4 participants
Similar to #6053