[ 
https://issues.apache.org/jira/browse/BEAM-11411?focusedWorklogId=539565&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-539565
 ]

ASF GitHub Bot logged work on BEAM-11411:
-----------------------------------------

                Author: ASF GitHub Bot
            Created on: 22/Jan/21 05:42
            Start Date: 22/Jan/21 05:42
    Worklog Time Spent: 10m 
      Work Description: 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.

##########
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 used 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]


Issue Time Tracking
-------------------

    Worklog Id:     (was: 539565)
    Time Spent: 6h 50m  (was: 6h 40m)

> Example of E2E test for Pub/Sub environment
> -------------------------------------------
>
>                 Key: BEAM-11411
>                 URL: https://issues.apache.org/jira/browse/BEAM-11411
>             Project: Beam
>          Issue Type: Improvement
>          Components: examples-java
>            Reporter: Ilya Kozyrev
>            Priority: P2
>          Time Spent: 6h 50m
>  Remaining Estimate: 0h
>
> Kafka to Pub/Sub pipeline [example|https://github.com/apache/beam/pull/13112] 
> needs to have e2e tests. However, in Beam Testing Guide 
> [https://cwiki.apache.org/confluence/display/BEAM/Contribution+Testing+Guide] 
> , we have not found examples or documentation for e2e tests that require a 
> Pub/Sub environment can be set up.
> During the investigation and with the help of our reviewers we found possible 
> approaches for our issue. 
> For Pub/Sub, e2e test could use [Pub/Sub 
> emulator|https://cloud.google.com/pubsub/docs/emulator]
> These e2e test examples can be referenced in the 
> [https://cwiki.apache.org/confluence/display/BEAM/Contribution+Testing+Guide] 
> to serve examples of how customers may approach e2e testing of pipelines in 
> their environments.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Reply via email to