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

7228 /sql REST call now compatible with auth #7230

Merged
merged 1 commit into from
Jul 29, 2021
Merged

7228 /sql REST call now compatible with auth #7230

merged 1 commit into from
Jul 29, 2021

Conversation

joshigaurava
Copy link
Contributor

Description

/sql REST call was using Axios configuration without appropriate auth headers. Once the configuration is provided with auth headers (when available), the call executes fine.

Also now, upon any network error, the UI will fail exactly the way it fails for any other network call. Graceful failure, if not available, needs to be implemented separately throughout the app.

@codecov-commenter
Copy link

codecov-commenter commented Jul 29, 2021

Codecov Report

Merging #7230 (63ca5f9) into master (573651b) will increase coverage by 0.07%.
The diff coverage is n/a.

❗ Current head 63ca5f9 differs from pull request most recent head c1561dc. Consider uploading reports for the commit c1561dc to get more accurate results
Impacted file tree graph

@@             Coverage Diff              @@
##             master    #7230      +/-   ##
============================================
+ Coverage     73.48%   73.56%   +0.07%     
  Complexity       92       92              
============================================
  Files          1508     1508              
  Lines         73951    73951              
  Branches      10685    10685              
============================================
+ Hits          54342    54400      +58     
+ Misses        16047    15991      -56     
+ Partials       3562     3560       -2     
Flag Coverage Δ
integration 41.94% <ø> (+0.02%) ⬆️
unittests 65.27% <ø> (+0.01%) ⬆️

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

Impacted Files Coverage Δ
...readers/constant/ConstantMVForwardIndexReader.java 14.28% <0.00%> (-28.58%) ⬇️
...ot/segment/local/customobject/MinMaxRangePair.java 75.86% <0.00%> (-24.14%) ⬇️
...impl/dictionary/FloatOffHeapMutableDictionary.java 72.34% <0.00%> (-5.32%) ⬇️
...core/query/executor/ServerQueryExecutorV1Impl.java 81.42% <0.00%> (-4.92%) ⬇️
...pinot/core/query/request/context/TimerContext.java 91.66% <0.00%> (-4.17%) ⬇️
...mpl/dictionary/DoubleOffHeapMutableDictionary.java 57.44% <0.00%> (-2.13%) ⬇️
...mpl/dictionary/StringOffHeapMutableDictionary.java 86.95% <0.00%> (-1.45%) ⬇️
...ation/function/MinMaxRangeAggregationFunction.java 87.50% <0.00%> (-1.25%) ⬇️
...a/org/apache/pinot/core/common/DataBlockCache.java 96.21% <0.00%> (-0.76%) ⬇️
...roker/requesthandler/BaseBrokerRequestHandler.java 71.28% <0.00%> (-0.22%) ⬇️
... and 19 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 573651b...c1561dc. Read the comment docs.

Copy link
Contributor

@apucher apucher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

appreciated

@apucher apucher merged commit 5e0dcff into apache:master Jul 29, 2021
@apucher
Copy link
Contributor

apucher commented Jul 30, 2021

addresses #7228

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.

Pinot UI doesn't set correct Auth header for queries
3 participants