kpatelatwork commented on a change in pull request #10822:
URL: https://github.com/apache/kafka/pull/10822#discussion_r655757673
##########
File path:
connect/runtime/src/test/java/org/apache/kafka/connect/util/clusters/EmbeddedConnectCluster.java
##########
@@ -559,6 +560,31 @@ public String endpointForResource(String resource) {
return url + resource;
}
+ /**
+ * Get the full URL of the endpoint that corresponds to the given REST
resource using a worker
+ * that is not running any tasks or connector instance for the
connectorName provided in the arguments
+ *
+ * @param resource the resource under the worker's admin endpoint
+ * @param connectorName the name of the connector
+ * @return the admin endpoint URL
+ * @throws ConnectException if no REST endpoint is available
+ */
+ public String endpointForWorkerRunningNoResourceForConnector(String
resource, String connectorName) {
Review comment:
@rhauch I think it's a good idea but I saw that
org.apache.kafka.connect.util.clusters.EmbeddedConnectCluster#endpointForResource
method is using the same pattern and if I change it then I will have to change
a lot of classes not related to this PR and ultimately in the test I will have
to check for Optional.present and do an assert. The situation for not finding
a worker is rare and doing empty check and assert in all places will be a lot
of copy/paste whereas throwing an exception here is centralizing this rare
situation. Do you think it's ok to leave it as is?
--
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:
us...@infra.apache.org
