TheNeuralBit commented on a change in pull request #13636:
URL: https://github.com/apache/beam/pull/13636#discussion_r562275044
##########
File path: examples/java/build.gradle
##########
@@ -56,6 +56,7 @@ dependencies {
compile library.java.vendored_guava_26_0_jre
compile library.java.kafka_clients
compile project(path: ":sdks:java:core", configuration: "shadow")
+ compile project(path: ":runners:direct-java", configuration: "shadow")
Review comment:
Took a closer look at this. It looks like the dependency is necessary
because `DirectOptions` is referenced in the new test. Could you make it a
`testCompile` dependency?
##########
File path:
examples/java/src/main/java/org/apache/beam/examples/complete/kafkatopubsub/kafka/consumer/Utils.java
##########
@@ -162,4 +165,11 @@ public static boolean isSslSpecified(KafkaToPubsubOptions
options) {
|| options.getKeystorePath() != null
|| options.getKeyPassword() != null;
}
+
+ public static Map<String, Object> parseKafkaConsumerConfig(String
kafkaConsumerConfig) {
+ return Arrays.stream(kafkaConsumerConfig.split(";"))
+ .map(s -> s.split("="))
+ .map(kv -> Pair.of(kv[0], kv[1]))
+ .collect(Collectors.toMap(Pair::getKey, Pair::getValue));
Review comment:
nit: If I were writing this I would probably combine these two lines and
avoid creating the `Pair`:
```suggestion
.collect(Collectors.toMap(kv -> kv[0], kv -> kv[1]));
```
If you prefer it with the Pair that's fine too.
One thing I think we _should_ change is make this a private static method in
`KafkaToPubsub`, since it's only user there.
----------------------------------------------------------------
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:
[email protected]
