You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2020/10/02 16:00:40 UTC
[GitHub] [kafka] tombentley opened a new pull request #9365: KAFKA-10566: Fix erroneous config usage warnings
tombentley opened a new pull request #9365:
URL: https://github.com/apache/kafka/pull/9365
Fix warn log messages caused by making HashMap copies of configs prior to using. This is not an ideal solution, but because the `Map`s are passed through `Configurable.configure(Map)` it's not possible to use another type (such as the `RecordingMap` type) directly.
----------------------------------------------------------------
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
[GitHub] [kafka] tombentley closed pull request #9365: KAFKA-10566: Fix erroneous config usage warnings
Posted by GitBox <gi...@apache.org>.
tombentley closed pull request #9365:
URL: https://github.com/apache/kafka/pull/9365
----------------------------------------------------------------
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
[GitHub] [kafka] tombentley commented on pull request #9365: KAFKA-10566: Fix erroneous config usage warnings
Posted by GitBox <gi...@apache.org>.
tombentley commented on pull request #9365:
URL: https://github.com/apache/kafka/pull/9365#issuecomment-705503807
@rajinisivaram @omkreddy did you have any thoughts about this? What I've done here isn't exactly pretty, but it was the simplest thing I could think of which would remove the erroneous warnings.
As an aside: I spent quite some time looking at how the `AbstractConfig` tracks usage in order to log about unknown config. TBH it's a pattern which, as here, doesn't always work well, because what's passed to `Configrable` is a `Map` so it's easy for people to forget that they need to track usage if they need to configure a plugin with something other than `AbstractConfig.values()`.
In some ways it would be better to be able to validate config parameters at the point of creating the `AbstractConfig`, but that would require lots of public API change to be able to build the overall `ConfigDef` (including all the plugins defined in the "root config"). As well as addressing the logging of unknown configs in a less flaky way, that would open the door for `describeConfigs` to be able to describe configs for plugins as well as the broker, for example. But it would be a lot of work if those were the only benefits. Thoughts?
----------------------------------------------------------------
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
[GitHub] [kafka] tombentley commented on pull request #9365: KAFKA-10566: Fix erroneous config usage warnings
Posted by GitBox <gi...@apache.org>.
tombentley commented on pull request #9365:
URL: https://github.com/apache/kafka/pull/9365#issuecomment-702817414
@rajinisivaram @omkreddy perhaps one of you could review?
----------------------------------------------------------------
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
[GitHub] [kafka] chia7712 commented on pull request #9365: KAFKA-10566: Fix erroneous config usage warnings
Posted by GitBox <gi...@apache.org>.
chia7712 commented on pull request #9365:
URL: https://github.com/apache/kafka/pull/9365#issuecomment-721617530
Is it similar to https://issues.apache.org/jira/browse/KAFKA-10090?
----------------------------------------------------------------
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
[GitHub] [kafka] tombentley commented on pull request #9365: KAFKA-10566: Fix erroneous config usage warnings
Posted by GitBox <gi...@apache.org>.
tombentley commented on pull request #9365:
URL: https://github.com/apache/kafka/pull/9365#issuecomment-721230361
Maybe @chia7712, @dajac or @mimaison could take a look at 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
[GitHub] [kafka] tombentley commented on pull request #9365: KAFKA-10566: Fix erroneous config usage warnings
Posted by GitBox <gi...@apache.org>.
tombentley commented on pull request #9365:
URL: https://github.com/apache/kafka/pull/9365#issuecomment-721626482
@chia7712 I think there's overlap, but I think this one removes additional warnings because it also propagates usage tracking in `SslFactory`. (It also provides a non-public abstraction to as least make it easier to identify the places where we end up doing this usage tracking, which might be useful should we ever figure out a better way of doing 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
[GitHub] [kafka] tombentley commented on pull request #9365: KAFKA-10566: Fix erroneous config usage warnings
Posted by GitBox <gi...@apache.org>.
tombentley commented on pull request #9365:
URL: https://github.com/apache/kafka/pull/9365#issuecomment-705503807
@rajinisivaram @omkreddy did you have any thoughts about this? What I've done here isn't exactly pretty, but it was the simplest thing I could think of which would remove the erroneous warnings.
As an aside: I spent quite some time looking at how the `AbstractConfig` tracks usage in order to log about unknown config. TBH it's a pattern which, as here, doesn't always work well, because what's passed to `Configrable` is a `Map` so it's easy for people to forget that they need to track usage if they need to configure a plugin with something other than `AbstractConfig.values()`.
In some ways it would be better to be able to validate config parameters at the point of creating the `AbstractConfig`, but that would require lots of public API change to be able to build the overall `ConfigDef` (including all the plugins defined in the "root config"). As well as addressing the logging of unknown configs in a less flaky way, that would open the door for `describeConfigs` to be able to describe configs for plugins as well as the broker, for example. But it would be a lot of work if those were the only benefits. Thoughts?
----------------------------------------------------------------
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
[GitHub] [kafka] tombentley commented on pull request #9365: KAFKA-10566: Fix erroneous config usage warnings
Posted by GitBox <gi...@apache.org>.
tombentley commented on pull request #9365:
URL: https://github.com/apache/kafka/pull/9365#issuecomment-708295751
@rajinisivaram @omkreddy @ijuma @mjsax please could someone take a look at this fix?
----------------------------------------------------------------
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