abhishekrb19 commented on code in PR #15485:
URL: https://github.com/apache/druid/pull/15485#discussion_r1422952440
##########
extensions-contrib/kafka-emitter/src/main/java/org/apache/druid/emitter/kafka/KafkaEmitterConfig.java:
##########
@@ -83,7 +87,8 @@ public KafkaEmitterConfig(
@Nullable @JsonProperty("request.topic") String requestTopic,
@Nullable @JsonProperty("segmentMetadata.topic") String
segmentMetadataTopic,
@JsonProperty("clusterName") String clusterName,
- @JsonProperty("producer.config") @Nullable Map<String, String>
kafkaProducerConfig
+ @JsonProperty("producer.config") @Nullable Map<String, String>
kafkaProducerConfig,
Review Comment:
What would happen if the type of existing `producer.config` is upgraded from
`Map<String, String>` to `Map<String, Object>`? I _think_ the type upgrade
should be safe considering `Object` is a superclass of `String`, but I'm not
certain it would work seamlessly without breaking compatibility for existing
deployments during rolling upgrades, rollbacks, etc.
Alternatively, if the type upgrade approach isn't feasible, we could
deprecate `producer.config Map<String, String>` in favor of a new config of
type `Map<String, Object>` that encompasses the dynamic config provider similar
to what you have here. This would be consistent with other areas of the system
like [kafka consumer
properties](https://github.com/apache/druid/blob/master/extensions-core/kafka-indexing-service/src/test/java/org/apache/druid/indexing/kafka/KafkaRecordSupplierTest.java#L718),
[iceberg catalog
properties](https://github.com/apache/druid/blob/master/extensions-contrib/druid-iceberg-extensions/src/main/java/org/apache/druid/iceberg/input/HiveIcebergCatalog.java#L78)
since it's documented here:
https://druid.apache.org/docs/latest/operations/dynamic-config-provider/
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
