[GitHub] [griffin] XiaoyuBD commented on pull request #587: Support http remote conf

[GitBox <gi...@apache.org>]

XiaoyuBD commented on pull request #587:
URL: https://github.com/apache/griffin/pull/587#issuecomment-738654894


   @wankunde Thank you for the review, it is really strange to add getData to HttpUtil.
   Also, I checked the code in HttpUtil and found the following problems:
   1. The `HttpUtil.postData()` and `HttpUtil.httpRequest()` has no usages.
   2. There is a bug in `HttpUtil.doHttpRequest()`. When method = 'PUT', after `case PUT_REGEX() =>`, the GET request is executed instead of PUT.
   3. The third parameter `params: Map[String, Object]` in `HttpUtil.doHttpRequest()` are not used and do not take effect.
   4. `HttpUtil.doHttpRequest()` only supports POST and "PUT" request. (GET and DELETE are not supported.)
   
   So my new commit is as follows:
   1. Remove `HttpUtil.postData()` and `HttpUtil.httpRequest()`, which has no usages.
   2. Reimplement` HttpUtil.doHttpRequest()`, enable use of third parameter `params: Map[String, Object]`, and support GET, POST, PUT, DELETE methods, and return status code and response string.
   3. ElasticSearchSink.scala, which calls HttpUtil.doHttpRequest(), has also been modified accordingly.
   
   Since the method is an external Http request, UT is not easy to implement. I tested it in our environment and it worked very well.
   


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