Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for Auth in controller requests in java query client #9230

Merged
merged 3 commits into from
Sep 7, 2022

Conversation

KKcorps
Copy link
Contributor

@KKcorps KKcorps commented Aug 17, 2022

The PR adds the support for auth and https when using fromController of java client.

The PR also does refactor some interfaces while maintaining backwards compatibility

  • The JsonAsyncHttpPinotClientTransportFactory is refactored to initiate all the configs such as headers, scheme etc. from Properties object itself. This is done so that end user doesn't have to create and pass a new transport object if they want to enable https or auth.
  • Support for passing controllerUrl directly instead of host and port so as to maintain similarity with other methods such as fromZookeeper, fromBrokerList etc.
  • Support for timeouts in controller requests

@KKcorps KKcorps requested a review from xiangfu0 August 17, 2022 10:10
@codecov-commenter
Copy link

codecov-commenter commented Aug 17, 2022

Codecov Report

Merging #9230 (dc885ff) into master (2f19683) will decrease coverage by 43.64%.
The diff coverage is 39.07%.

❗ Current head dc885ff differs from pull request most recent head 4247a1d. Consider uploading reports for the commit 4247a1d to get more accurate results

@@              Coverage Diff              @@
##             master    #9230       +/-   ##
=============================================
- Coverage     69.88%   26.23%   -43.65%     
+ Complexity     5008       44     -4964     
=============================================
  Files          1854     1845        -9     
  Lines         98871    98633      -238     
  Branches      15043    15011       -32     
=============================================
- Hits          69094    25875    -43219     
- Misses        24904    70194    +45290     
+ Partials       4873     2564     -2309     
Flag Coverage Δ
integration1 26.23% <39.07%> (+0.08%) ⬆️
integration2 ?
unittests1 ?
unittests2 ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...e/pinot/broker/broker/helix/BaseBrokerStarter.java 74.64% <ø> (-5.17%) ⬇️
...ot/broker/requesthandler/BrokerRequestHandler.java 0.00% <0.00%> (ø)
...main/java/org/apache/pinot/client/BrokerCache.java 0.00% <0.00%> (ø)
...pache/pinot/client/BrokerCacheUpdaterPeriodic.java 0.00% <0.00%> (ø)
...ava/org/apache/pinot/client/ConnectionFactory.java 33.33% <0.00%> (-27.39%) ⬇️
...he/pinot/client/ControllerBasedBrokerSelector.java 0.00% <0.00%> (ø)
...ata/manager/realtime/RealtimeTableDataManager.java 42.52% <0.00%> (-25.84%) ⬇️
...ava/org/apache/pinot/core/plan/FilterPlanNode.java 48.87% <0.00%> (-33.58%) ⬇️
.../java/org/apache/pinot/query/QueryEnvironment.java 0.00% <0.00%> (-87.50%) ⬇️
...ry/rules/PinotLogicalSortFetchEliminationRule.java 0.00% <0.00%> (ø)
... and 1373 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@KKcorps KKcorps merged commit 15e9398 into apache:master Sep 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants