MINOR: Cleanup Connect Module (3/n) #20156
Conversation
github-actions
bot
added
triage
m1a2st
left a comment
m1a2st
left a comment
There was a problem hiding this comment.
Thank @sjhajharia for this patch, left one comment
connect/runtime/src/test/java/org/apache/kafka/connect/integration/ConnectorHandle.java
Outdated
Show resolved
Hide resolved
|
Thanks @m1a2st for the comment. I have addressed the same at all locations where it is possible. Requesting a re-review. |
|
Adding @chia7712 to the PR as he kindly helped review a bunch of PRs in this area. |
...ct/runtime/src/test/java/org/apache/kafka/connect/integration/OffsetsApiIntegrationTest.java
Outdated
Show resolved
Hide resolved
...ct/runtime/src/test/java/org/apache/kafka/connect/integration/OffsetsApiIntegrationTest.java
Outdated
Show resolved
Hide resolved
...ct/runtime/src/test/java/org/apache/kafka/connect/integration/OffsetsApiIntegrationTest.java
Outdated
Show resolved
Hide resolved
...test/java/org/apache/kafka/connect/integration/RebalanceSourceConnectorsIntegrationTest.java
Show resolved
Hide resolved
|
Thanks @chia7712 for the initial reviews. I have addressed the same. |
.../apache/kafka/connect/connector/policy/PrincipalConnectorClientConfigOverridePolicyTest.java
Outdated
Show resolved
Hide resolved
| public List<Map<String, String>> taskConfigs(int maxTasks) { | ||
| block.maybeBlockOn(CONNECTOR_TASK_CONFIGS); | ||
| return Collections.singletonList(Collections.emptyMap()); | ||
| return List.of(Map.of()); |
There was a problem hiding this comment.
Could you please update line#790 also?
@Override
public List<Map<String, String>> taskConfigs(int maxTasks) {
return IntStream.range(0, maxTasks)
.mapToObj(i -> new HashMap<>(props))
.collect(Collectors.toList());
}There was a problem hiding this comment.
Are you referencing to get rid of .collect(Collectors.toList()) in favour of .toList()? if so, then that is not possible here as we are using HashMap while the return type needs to be Map.
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/WorkerSinkTaskTest.java
Outdated
Show resolved
Hide resolved
| public void shouldValidateAllVerificationAlgorithms() { | ||
| List<String> algorithms = | ||
| new ArrayList<>(Arrays.asList("HmacSHA1", "HmacSHA256", "HmacMD5", "bad-algorithm")); | ||
| new ArrayList<>(List.of("HmacSHA1", "HmacSHA256", "HmacMD5", "bad-algorithm")); |
There was a problem hiding this comment.
Could we make algorithms immutable in this test?
There was a problem hiding this comment.
I have updated the same with a different approach. Pls review if that makes sense.
...untime/src/test/java/org/apache/kafka/connect/runtime/distributed/DistributedHerderTest.java
Show resolved
Hide resolved
...sources/test-plugins/read-version-from-resource-v1/test/plugins/ReadVersionFromResource.java
Outdated
Show resolved
Hide resolved
|
@sjhajharia could you please take a look at the failed tests? |
|
Thanks @chia7712 |
chia7712
left a comment
chia7712
left a comment
There was a problem hiding this comment.
@sjhajharia thanks for fixing the failure. I have one last comment.
|
|
||
| when(configStore.snapshot()).thenReturn(SNAPSHOT); | ||
| assertEquals(Collections.singleton(CONN1), new HashSet<>(herder.connectors())); | ||
| assertEquals(Set.of(CONN1), new HashSet<>(herder.connectors())); |
There was a problem hiding this comment.
assertEquals(Set.of(CONN1), Set.copyOf(herder.connectors()));
WDYT?
There was a problem hiding this comment.
Makes sense. Fixed it.
3 participants
Now that Kafka support Java 17, this PR makes some changes in connect
module. The changes in this PR are limited to only some files. A future
PR(s) shall follow.
The changes mostly include:
Modules target: runtime/src/test
Reviewers: Ken Huang s7133700@gmail.com, Chia-Ping Tsai chia7712@gmail.com