luchunliang commented on a change in pull request #3162:
URL: https://github.com/apache/incubator-inlong/pull/3162#discussion_r827613942



##########
File path: 
inlong-dataproxy/dataproxy-source/src/main/java/org/apache/inlong/dataproxy/source/ServerMessageHandler.java
##########
@@ -525,7 +529,8 @@ private void responsePackage(ChannelHandlerContext ctx, 
Map<String, String> comm
             Channel remoteChannel,
             SocketAddress remoteSocketAddress,
             MsgType msgType) throws Exception {
-        if (!commonAttrMap.containsKey("isAck") || 
"true".equals(commonAttrMap.get("isAck"))) {
+        if (!commonAttrMap.containsKey(AttributeConstants.MESSAGE_IS_ACK)
+                || 
"true".equals(commonAttrMap.get(AttributeConstants.MESSAGE_IS_ACK))) {

Review comment:
       It is better. And 
   String isAck = commonAttrMap.getOrDefault(AttributeConstants.MESSAGE_IS_ACK, 
Boolean.TRUE.toString());
   if (Boolean.TRUE.toString().equals(isAck)) {

##########
File path: 
inlong-dataproxy/dataproxy-source/src/main/java/org/apache/inlong/dataproxy/sink/pulsar/PulsarClientService.java
##########
@@ -246,6 +245,9 @@ public boolean sendMessage(int poolIndex, String topic, 
Event event,
      * @param orderEvent orderEvent
      */
     private void sendResponse(OrderEvent orderEvent) {
+        if 
("false".equals(orderEvent.getHeaders().get(AttributeConstants.MESSAGE_IS_ACK)))
 {
+            return;

Review comment:
       How about 
"orderEvent.getHeaders().get(AttributeConstants.MESSAGE_IS_ACK))==null"?
   It need to return directly also.

##########
File path: 
inlong-common/src/main/java/org/apache/inlong/common/reporpter/AbstractReporter.java
##########
@@ -101,14 +101,15 @@ public Response syncReportData(T data, String serverUrl) 
throws Exception {
             return null;
         }
         HttpPost httpPost = new HttpPost(serverUrl);
+        String returnStr = null;
         try {
             StringEntity stringEntity = new StringEntity(gson.toJson(data));
             stringEntity.setContentType(AGENT_HTTP_APPLICATION_JSON);
             httpPost.setEntity(stringEntity);
-            String returnStr = executeHttpPost(httpPost);
+            returnStr = executeHttpPost(httpPost);
             return parse(returnStr);
         } catch (Exception e) {
-            LOGGER.error("syncReportData has exception e = {}", e);
+            LOGGER.error("syncReportData has exception returnStr = {}, e:", 
returnStr, e);
             throw e;

Review comment:
       It is better to return a Response with error code.

##########
File path: 
inlong-dataproxy/dataproxy-source/src/main/java/org/apache/inlong/dataproxy/source/ServerMessageHandler.java
##########
@@ -442,6 +442,10 @@ private void formatMessagesAndSend(ChannelHandlerContext 
ctx, Map<String, String
                     headers.put(AttributeConstants.DATA_TIME, 
String.valueOf(System.currentTimeMillis()));
                 }
 
+                if 
("false".equals(commonAttrMap.get(AttributeConstants.MESSAGE_IS_ACK))) {
+                    headers.put(AttributeConstants.MESSAGE_IS_ACK, "false");
+                }

Review comment:
       How about 
"orderEvent.getHeaders().get(AttributeConstants.MESSAGE_IS_ACK))==null" and 
other value?




-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to