You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2022/02/24 22:24:07 UTC

[GitHub] [pinot] yupeng9 opened a new issue #8246: Java client for Controller/Broker/Server REST APIs

yupeng9 opened a new issue #8246:
URL: https://github.com/apache/pinot/issues/8246


   It will be very helpful to have a library for the common REST APIs for Controller/Broker/Server. There are a number of existing methods/helpers in the code repo, such as in the test utils or [connector](https://github.com/apache/pinot/pull/8233#discussion_r814287411) for this purpose. But those are adhoc and not comprehensive. 
   
    


-- 
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: commits-unsubscribe@pinot.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] Jackie-Jiang commented on issue #8246: Java client for Controller/Broker/Server REST APIs

Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on issue #8246:
URL: https://github.com/apache/pinot/issues/8246#issuecomment-1050422762


   +1
   
   `ControllerRequestURLBuilder` contains most of the APIs. We can probably wrap it into a library


-- 
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: commits-unsubscribe@pinot.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] walterddr commented on issue #8246: Java client for Controller/Broker/Server REST APIs

Posted by GitBox <gi...@apache.org>.
walterddr commented on issue #8246:
URL: https://github.com/apache/pinot/issues/8246#issuecomment-1066165696


   #8329 seems to work pretty well as a POC: here is my adjusted plan based on the POC
   
   Problem
   ===
   Currently there are many ad-hoc usage/utils that makes http call to Pinot components, to name a few
   - `ControllerTestUtils` that uses `URLConnection` to Controller 
     - also uses `HTTPClient` from Apache HTTP module for 2 of the multi-part request
   - FileUploadDownloadClient that uses Apache Common `HTTPClient` with SSLContext support
   - other standalone modules such as Pinot spark/flink connector that uses its own HTTPUtils
   
   There are discrepancies regarding (1) what's the format of the send data / return; (2) what type of return code throws exception or encapsulate the exception in the response code; (3) what default header is being used. 
   
   What's more, there are inefficiency in HTTPClient usage: in general HTTPClient should be used as a singleton and leverage the underlying resource reused benefit. 
   
   Proposal
   ===
   Propose to 
   1. add HTTPClient in pinot-common that provides generic get/post/put/delete method to RestAPI and parsing response objects/status code.
       - for advance usage such as file upload download, also provide the API to return closable http response stream.
   2. ControllerRequestClient that utilizes the HTTPClient singleton to make specific requests such as getSchema, getTableConfig, etc.
       - later on we can extend to Broker/Server APIs.
   3. Replace any http request utils with the client construct (with a static reference to the singelton HTTPClient)
   
   
   


-- 
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: commits-unsubscribe@pinot.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] walterddr commented on issue #8246: Java client for Controller/Broker/Server REST APIs

Posted by GitBox <gi...@apache.org>.
walterddr commented on issue #8246:
URL: https://github.com/apache/pinot/issues/8246#issuecomment-1059393420


   see: #8293 for demonstration of the refactoring


-- 
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: commits-unsubscribe@pinot.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] mcvsubbu commented on issue #8246: Java client for Controller/Broker/Server REST APIs

Posted by GitBox <gi...@apache.org>.
mcvsubbu commented on issue #8246:
URL: https://github.com/apache/pinot/issues/8246#issuecomment-1051074825


   Agreed that this will be a useful thing.  
   
   We will just have to pay more attention to a few things then:
   - Keeping the existing ReST APIs compatible
   - Keeping in mind that the APIs cold be issued programmatically, so make sure they work in corner cases (like during upgrades, etc.) correctly.
   - Keeping the client and controller in sync.
   
   As long as we are wiling to bear the cost, I am all for this  feature.


-- 
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: commits-unsubscribe@pinot.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] walterddr commented on issue #8246: Java client for Controller/Broker/Server REST APIs

Posted by GitBox <gi...@apache.org>.
walterddr commented on issue #8246:
URL: https://github.com/apache/pinot/issues/8246#issuecomment-1058636542


   +1 for this proposal. I am also working on factoring out the helper functions from ControllerTestUtils. 
   
   Here is what I proposed to do, please kindly let me know if this is the right track:
   
   1. summarized all the helper/utils we currently have. (in addition to the ones discussed above, many of the `*RestResource` in controller/broker have internal static functions to do send request and parse responses)
   2. proposal to add different level of abstractions. 
       - HTTPUtils in pinot-common (for constructing pinot get/post/put/delete to RestAPI and parsing response objects)
       - ControllerUtils that utilizes the HTTPUtils to make specific requests such as getSchema, getTableConfig, etc.
       - JavaClient that wraps around the ControllerUtils and is cluster-context aware (e.g. able to configure controller URL during start up and keeps tracking the active controller)
   3. tests to ensure that they are RestAPI compatible; and ensure the client API and controller API are in sync.
   
   Any suggestions? I will create an example PR to demonstrate what I meant soon


-- 
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: commits-unsubscribe@pinot.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] walterddr commented on issue #8246: Java client for Controller/Broker/Server REST APIs

Posted by GitBox <gi...@apache.org>.
walterddr commented on issue #8246:
URL: https://github.com/apache/pinot/issues/8246#issuecomment-1081981729


   update on status: with #8329 and #8390 contributed. item 1 and 3 is almost finished. 
   we can now: add functionalities to `ControllerRequestClient` and create `Server/BrokerRequestClients`. 
   
   there are other places where either `java.net.*` or `apache.http.*` is directly used, I propose to create a separate ticket to track the migration of these usages as they are (1) many and scattered around the codebase and might not make sense to migrate all; (2) mostly will be migrated over when security/auth is enabled for example QueryRunner now doesn't support auth token. 


-- 
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: commits-unsubscribe@pinot.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] walterddr edited a comment on issue #8246: Java client for Controller/Broker/Server REST APIs

Posted by GitBox <gi...@apache.org>.
walterddr edited a comment on issue #8246:
URL: https://github.com/apache/pinot/issues/8246#issuecomment-1059393420


   see: #8293 for demonstration of the refactoring
   
   [update] #8329 is the new POC, for several reasons
   - comparing with using `URLConnection`, HTTPClient seems a better choice (see: https://www.mocklab.io/blog/which-java-http-client-should-i-use-in-2020/)
   - HTTPClient is recommended to be reused (see: https://hc.apache.org/httpclient-legacy/performance.html)


-- 
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: commits-unsubscribe@pinot.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org