[GitHub] [kafka] yashmayya commented on a diff in pull request #12984: KAFKA-14455: Kafka Connect create and update REST APIs should surface failures while writing to the config topic

[GitBox]

yashmayya commented on code in PR #12984:
URL: https://github.com/apache/kafka/pull/12984#discussion_r1060149266


##########
connect/runtime/src/main/java/org/apache/kafka/connect/storage/KafkaConfigBackingStore.java:
##########
@@ -712,8 +733,16 @@ KafkaBasedLog<String, byte[]> 
setupAndCreateKafkaBasedLog(String topic, final Wo
     }
 
     private void sendPrivileged(String key, byte[] value) {
+        sendPrivileged(key, value, null);
+    }
+
+    private void sendPrivileged(String key, byte[] value, Callback<Void> 
callback) {
         if (!usesFencableWriter) {
-            configLog.send(key, value);

Review Comment:
   I agree that terminating the producer callbacks in the 
`KafkaConfigBackingStore` itself would look cleaner. However, if we want to 
surface producer errors to users via the REST API responses (which is the goal 
for this PR), we would need to complete the request callbacks either directly 
or via an intermediary callback (also see [this 
comment](https://github.com/apache/kafka/pull/12984#discussion_r1046720576)) in 
the `KafkaConfigBackingStore`  (both of which would involve changing the method 
signatures to accept callbacks) due to the way that the REST API responses are 
generated using `FutureCallback` in `ConnectorsResource`.



-- 
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-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org