You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by GitBox <gi...@apache.org> on 2019/07/18 08:42:52 UTC

[GitHub] [incubator-druid] satybald commented on a change in pull request #8078: Upgrade Kafka library for kafka-lookup module

satybald commented on a change in pull request #8078: Upgrade Kafka library for kafka-lookup module
URL: https://github.com/apache/incubator-druid/pull/8078#discussion_r304801214
 
 

 ##########
 File path: extensions-core/kafka-extraction-namespace/src/main/java/org/apache/druid/query/lookup/KafkaLookupExtractorFactory.java
 ##########
 @@ -413,4 +353,47 @@ AtomicLong getDoubleEventCount()
   {
     return future;
   }
+
+  private void verifyKafkaProperties()
+  {
+    if (kafkaProperties.containsKey(ConsumerConfig.GROUP_ID_CONFIG)) {
+      throw new IAE(
+              "Cannot set kafka property [group.id]. Property is randomly generated for you. Found [%s]",
+              kafkaProperties.get(ConsumerConfig.GROUP_ID_CONFIG)
+      );
+    }
+    if (kafkaProperties.containsKey(ConsumerConfig.AUTO_OFFSET_RESET_CONFIG)) {
+      throw new IAE(
+              "Cannot set kafka property [auto.offset.reset]. Property will be forced to [smallest]. Found [%s]",
+              kafkaProperties.get(ConsumerConfig.AUTO_OFFSET_RESET_CONFIG)
+      );
+    }
+    Preconditions.checkNotNull(
+            kafkaProperties.get(ConsumerConfig.BOOTSTRAP_SERVERS_CONFIG),
+            "bootstrap.servers required property"
+    );
+  }
+
+  // Overridden in tests
+  Consumer<String, String> getConsumer()
+  {
+    // Workaround for Kafka String Serializer could not be found
+    // Adopted from - https://stackoverflow.com/a/54118010/2586315
 
 Review comment:
   Thank you for review my contribution. I don't hold any degree in law but I'd argue that this case falls under Fair use. I listed SO link to justify this approach as we had such issue when I tested it with druid cluster. At the same time, I can argue that it's a common approach for dealing with such class loading issues.
   
   I'll take a look at the Kafka indexing druid module to not deal with legal issues.
   
   https://en.wikipedia.org/wiki/Fair_use 

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org