You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@shardingsphere.apache.org by GitBox <gi...@apache.org> on 2020/08/31 06:29:01 UTC

[GitHub] [shardingsphere-elasticjob] TeslaCN opened a new pull request #1432: Improve HTTP job executor

TeslaCN opened a new pull request #1432:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1432


   Fixes #1431 
   
   Changes proposed in this pull request:
   - Log the response body in WARN level if HTTP request is not OK.
   - Replace explicit close with try-with-resource.
   - Adjust dependencies scope and add optional logger dependency for test logging.
   


----------------------------------------------------------------
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] [shardingsphere-elasticjob] TeslaCN commented on a change in pull request #1432: Improve HTTP job executor

Posted by GitBox <gi...@apache.org>.
TeslaCN commented on a change in pull request #1432:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1432#discussion_r479937610



##########
File path: elasticjob-executor/elasticjob-executor-type/elasticjob-http-executor/src/main/java/org/apache/shardingsphere/elasticjob/http/executor/HttpJobExecutor.java
##########
@@ -116,6 +118,10 @@ private boolean isWriteMethod(final String method) {
         return Arrays.asList("POST", "PUT", "DELETE").contains(method.toUpperCase());
     }
     
+    private boolean isRequestSucceed(final int httpStatusCode) {
+        return HttpURLConnection.HTTP_BAD_REQUEST > httpStatusCode;

Review comment:
       This condition is refer to `sun.net.www.protocol.http.HttpURLConnection#getErrorStream`, which implemented `java.net.HttpURLConnection`.




----------------------------------------------------------------
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] [shardingsphere-elasticjob] Technoboy- merged pull request #1432: Improve HTTP job executor

Posted by GitBox <gi...@apache.org>.
Technoboy- merged pull request #1432:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1432


   


----------------------------------------------------------------
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] [shardingsphere-elasticjob] Technoboy- commented on a change in pull request #1432: Improve HTTP job executor

Posted by GitBox <gi...@apache.org>.
Technoboy- commented on a change in pull request #1432:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1432#discussion_r479928881



##########
File path: elasticjob-executor/elasticjob-executor-type/elasticjob-http-executor/src/main/java/org/apache/shardingsphere/elasticjob/http/executor/HttpJobExecutor.java
##########
@@ -116,6 +118,10 @@ private boolean isWriteMethod(final String method) {
         return Arrays.asList("POST", "PUT", "DELETE").contains(method.toUpperCase());
     }
     
+    private boolean isRequestSucceed(final int httpStatusCode) {
+        return HttpURLConnection.HTTP_BAD_REQUEST > httpStatusCode;

Review comment:
       Why not use 'HttpURLConnection.HTTP_OK == httpStatusCode' to accurate the result ?




----------------------------------------------------------------
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