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 2020/04/30 22:44:02 UTC
[GitHub] [druid] JulianJaffePinterest commented on a change in pull request #9693: Allow bootstrap.servers to be provided for Kafka ingestion.
JulianJaffePinterest commented on a change in pull request #9693:
URL: https://github.com/apache/druid/pull/9693#discussion_r418330294
##########
File path: docs/development/extensions-core/kafka-ingestion.md
##########
@@ -194,7 +194,7 @@ For Concise bitmaps:
|-----|----|-----------|--------|
|`topic`|String|The Kafka topic to read from. This must be a specific topic as topic patterns are not supported.|yes|
|`inputFormat`|Object|[`inputFormat`](../../ingestion/data-formats.md#input-format) to specify how to parse input data. See [the below section](#specifying-data-format) for details about specifying the input format.|yes|
-|`consumerProperties`|Map<String, Object>|A map of properties to be passed to the Kafka consumer. This must contain a property `bootstrap.servers` with a list of Kafka brokers in the form: `<BROKER_1>:<PORT_1>,<BROKER_2>:<PORT_2>,...`. For SSL connections, the `keystore`, `truststore` and `key` passwords can be provided as a [Password Provider](../../operations/password-provider.md) or String password.|yes|
+|`consumerProperties`|Map<String, Object>|A map of properties to be passed to the Kafka consumer. This must contain a property `bootstrap.servers` with a list of Kafka brokers in the form: `<BROKER_1>:<PORT_1>,<BROKER_2>:<PORT_2>,...`, which may be either provided as a [Password Provider](../../operations/password-provider.md) or as a String. For SSL connections, the `keystore`, `truststore` and `key` passwords can be provided as a [Password Provider](../../operations/password-provider.md) or String password.|yes|
Review comment:
Yeah, part of #9351 is moving to `CredentialProvider` from `PasswordProvider`, which seems like a better term for this use case as well. I suppose eventually it might make sense to just go with `Provider` or the like if we want to allow any field to be provided at runtime.
##########
File path: extensions-core/kafka-indexing-service/src/test/java/org/apache/druid/indexing/kafka/supervisor/KafkaSupervisorIOConfigTest.java
##########
@@ -171,7 +171,7 @@ public void testSerdeForConsumerPropertiesWithPasswords() throws Exception
String jsonStr = "{\n"
+ " \"type\": \"kafka\",\n"
+ " \"topic\": \"my-topic\",\n"
- + " \"consumerProperties\": {\"bootstrap.servers\":\"localhost:9092\",\n"
+ + " \"consumerProperties\": {\"bootstrap.servers\":{\"type\": \"default\", \"password\": \"localhost:9092\"},\n"
Review comment:
Ah that's a good point (in order to get the DefaultPasswordProvider to serialize back to a simple string I think we also need to add `@JsonTypeInfo(use = JsonTypeInfo.Id.NONE)` so it won't serialize the type info). The tricky piece here is not breaking the `PasswordProviderRedactionMixIn`, as it seems that `@JsonValue` takes precedence over `@JsonIgnore`, even when I set `@JsonValue(false)` in the redaction mixin. I'll whip up a custom serializer to handle this.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org