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 2021/09/21 15:28:40 UTC
[GitHub] [kafka] C0urante opened a new pull request #11349: MINOR: Fix use of ConfigException in AbstractConfig class
C0urante opened a new pull request #11349:
URL: https://github.com/apache/kafka/pull/11349
The two-arg variant is intended to take a property name and value, not an exception message and a cause.
### Committer Checklist (excluded from commit message)
- [ ] Verify design and implementation
- [ ] Verify test coverage and CI build status
- [ ] Verify documentation (including upgrade notes)
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] tombentley merged pull request #11349: MINOR: Fix use of ConfigException in AbstractConfig class
Posted by GitBox <gi...@apache.org>.
tombentley merged pull request #11349:
URL: https://github.com/apache/kafka/pull/11349
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] tombentley commented on pull request #11349: MINOR: Fix use of ConfigException in AbstractConfig class
Posted by GitBox <gi...@apache.org>.
tombentley commented on pull request #11349:
URL: https://github.com/apache/kafka/pull/11349#issuecomment-928892539
Failing test is unrelated.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] C0urante commented on a change in pull request #11349: MINOR: Fix use of ConfigException in AbstractConfig class
Posted by GitBox <gi...@apache.org>.
C0urante commented on a change in pull request #11349:
URL: https://github.com/apache/kafka/pull/11349#discussion_r715771113
##########
File path: clients/src/main/java/org/apache/kafka/common/config/AbstractConfig.java
##########
@@ -577,13 +577,17 @@ public void logUnused() {
configProviderInstances.put(entry.getKey(), provider);
} catch (ClassNotFoundException e) {
log.error("ClassNotFoundException exception occurred: " + entry.getValue());
- throw new ConfigException("Invalid config:" + entry.getValue() + " ClassNotFoundException exception occurred", e);
+ throw new ConfigException(providerClassProperty(entry.getKey()), entry.getValue(), "ClassNotFoundException exception occurred");
Review comment:
Good point; I've updated the log line above to include the entire stack trace. It'd be nice if there were a variant of `ConfigException` that took in a cause argument but there is none at the moment and it's probably not worth it to add one just for this use case.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] tombentley commented on pull request #11349: MINOR: Fix use of ConfigException in AbstractConfig class
Posted by GitBox <gi...@apache.org>.
tombentley commented on pull request #11349:
URL: https://github.com/apache/kafka/pull/11349#issuecomment-928893384
Thanks @C0urante!
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] tombentley commented on a change in pull request #11349: MINOR: Fix use of ConfigException in AbstractConfig class
Posted by GitBox <gi...@apache.org>.
tombentley commented on a change in pull request #11349:
URL: https://github.com/apache/kafka/pull/11349#discussion_r715452885
##########
File path: clients/src/main/java/org/apache/kafka/common/config/AbstractConfig.java
##########
@@ -577,13 +577,17 @@ public void logUnused() {
configProviderInstances.put(entry.getKey(), provider);
} catch (ClassNotFoundException e) {
log.error("ClassNotFoundException exception occurred: " + entry.getValue());
- throw new ConfigException("Invalid config:" + entry.getValue() + " ClassNotFoundException exception occurred", e);
+ throw new ConfigException(providerClassProperty(entry.getKey()), entry.getValue(), "ClassNotFoundException exception occurred");
Review comment:
What if the class that's not found isn't the config provider but some dependency of it? Because the CNFE message isn't logged nor passed to the `ConfigException` this info would be lost.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] tombentley merged pull request #11349: MINOR: Fix use of ConfigException in AbstractConfig class
Posted by GitBox <gi...@apache.org>.
tombentley merged pull request #11349:
URL: https://github.com/apache/kafka/pull/11349
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] tombentley commented on pull request #11349: MINOR: Fix use of ConfigException in AbstractConfig class
Posted by GitBox <gi...@apache.org>.
tombentley commented on pull request #11349:
URL: https://github.com/apache/kafka/pull/11349#issuecomment-928892539
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org