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:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
