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/02/03 18:00:17 UTC
[GitHub] [kafka] Cyril-Engels opened a new pull request #10040: KAFKA-12259: Use raw config to infer connector type when returning connect status response
Cyril-Engels opened a new pull request #10040:
URL: https://github.com/apache/kafka/pull/10040
Addressed [KAFKA-12259](https://issues.apache.org/jira/browse/KAFKA-12259).
Problem: when calling the connect status endpoint, a 500 error is returned, e.g.
```
{
"error_code": 500,
"message": "Could not read properties from file /tmp/somefile.properties"
}
```
when any of the connector's has an exception from the config provider. This is because the `AbstractHerder` is trying to use resolved config. However, only the `connector.class` is needed from the config to infer if this connector is of source or sink type. The endpoint should still return the status of the connector instead of a 500 error.
This change uses raw config from the config backing store to retrieve the connector class to avoid any variable resolution.
Unit tests have been updated to reflect this change.
when any of the connector's has an exception from the config provider.
### 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] kkonstantine merged pull request #10040: KAFKA-12259: Use raw config to infer connector type when returning connect status response
Posted by GitBox <gi...@apache.org>.
kkonstantine merged pull request #10040:
URL: https://github.com/apache/kafka/pull/10040
----------------------------------------------------------------
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] kkonstantine commented on a change in pull request #10040: KAFKA-12259: Use raw config to infer connector type when returning connect status response
Posted by GitBox <gi...@apache.org>.
kkonstantine commented on a change in pull request #10040:
URL: https://github.com/apache/kafka/pull/10040#discussion_r569658355
##########
File path: connect/runtime/src/main/java/org/apache/kafka/connect/runtime/AbstractHerder.java
##########
@@ -284,7 +284,8 @@ public ConnectorStateInfo connectorStatus(String connName) {
Collections.sort(taskStates);
- Map<String, String> conf = config(connName);
+ final ClusterConfigState configState = configBackingStore.snapshot();
Review comment:
I'm suspecting we might be able to skip the `snapshot` here. Since we are asking for a specific connector and since we check the status topic before, it's highly probable that the connector config is already in the `configState` that has been snapshotted in distributed or standalone herders.
Should we just change the implementations of `AbstractHerder#config` to return the raw config?
##########
File path: connect/runtime/src/main/java/org/apache/kafka/connect/runtime/AbstractHerder.java
##########
@@ -284,7 +284,8 @@ public ConnectorStateInfo connectorStatus(String connName) {
Collections.sort(taskStates);
- Map<String, String> conf = config(connName);
Review comment:
we are removing the single use of this method.
Either we should remove it completely (including its implementations) or we could choose to extend it to return the raw config upon request. Eg. change it to `protected abstract Map<String, String> config(String connName, boolean returnRawConfig);`
`config(connName, true)` still won't have a use but maybe that's an ok exception for now. Or we can change it to `rawConfig(String connName)`.
See my comment below about whether we need to create a shallow copy of the config cluster state with the `snapshot` method.
----------------------------------------------------------------
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