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/04/23 06:50:02 UTC
[GitHub] [kafka] kkonstantine commented on a change in pull request #8536: KAFKA-9883: Add better error message when REST API forwards a request and leader is not known
kkonstantine commented on a change in pull request #8536:
URL: https://github.com/apache/kafka/pull/8536#discussion_r413549161
##########
File path: connect/runtime/src/main/java/org/apache/kafka/connect/runtime/rest/resources/ConnectorsResource.java
##########
@@ -341,7 +341,15 @@ private void checkAndPutConnectorConfigName(String connectorName, Map<String, St
// this gives two total hops to resolve the request before giving up.
boolean recursiveForward = forward == null;
RequestTargetException targetException = (RequestTargetException) cause;
- String forwardUrl = UriBuilder.fromUri(targetException.forwardUrl())
+ String forwardedUrl = targetException.forwardUrl();
+ if (forwardedUrl == null) {
+ // the target didn't know of the leader at this moment.
+ // we don't, it probably means that a rebalance has taken place.
Review comment:
this second comment line doesn't read very clearly for me.
who's "we" in this case?
wouldn't `this probably means that a rebalance has taken place` be as good?
##########
File path: connect/runtime/src/main/java/org/apache/kafka/connect/runtime/rest/resources/ConnectorsResource.java
##########
@@ -341,7 +341,15 @@ private void checkAndPutConnectorConfigName(String connectorName, Map<String, St
// this gives two total hops to resolve the request before giving up.
boolean recursiveForward = forward == null;
RequestTargetException targetException = (RequestTargetException) cause;
- String forwardUrl = UriBuilder.fromUri(targetException.forwardUrl())
+ String forwardedUrl = targetException.forwardUrl();
+ if (forwardedUrl == null) {
+ // the target didn't know of the leader at this moment.
+ // we don't, it probably means that a rebalance has taken place.
+ throw new ConnectRestException(Response.Status.CONFLICT.getStatusCode(),
+ "Cannot complete request momentarily due no known leader URL, "
Review comment:
nit: due _to_ no known leader URL ... or similar
----------------------------------------------------------------
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