You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "jayeshchoudhary (via GitHub)" <gi...@apache.org> on 2023/03/07 15:18:41 UTC

[GitHub] [pinot] jayeshchoudhary opened a new pull request, #10389: feat(ui): persist auth details

jayeshchoudhary opened a new pull request, #10389:
URL: https://github.com/apache/pinot/pull/10389

   Instructions:
   1. The PR has to be tagged with at least one of the following labels (*):
      1. `feature`
      2. `bugfix`
      3. `performance`
      4. `ui`
      5. `backward-incompat`
      6. `release-notes` (**)
   2. Remove these instructions before publishing the PR.
    
   (*) Other labels to consider:
   - `testing`
   - `dependencies`
   - `docker`
   - `kubernetes`
   - `observability`
   - `security`
   - `code-style`
   - `extension-point`
   - `refactor`
   - `cleanup`
   
   (**) Use `release-notes` label for scenarios like:
   - New configuration options
   - Deprecation of configurations
   - Signature changes to public methods/interfaces
   - New plugins added or old plugins removed
   


-- 
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] jayeshchoudhary commented on a diff in pull request #10389: feat(ui): persist oidc auth details

Posted by "jayeshchoudhary (via GitHub)" <gi...@apache.org>.
jayeshchoudhary commented on code in PR #10389:
URL: https://github.com/apache/pinot/pull/10389#discussion_r1134070826


##########
pinot-controller/src/main/resources/app/App.tsx:
##########
@@ -18,48 +18,42 @@
  */
 
 import * as React from 'react';
-import * as ReactDOM from 'react-dom';
-import { CircularProgress, createStyles, makeStyles, MuiThemeProvider } from '@material-ui/core';
-import { Switch, Route, HashRouter as Router, Redirect } from 'react-router-dom';
-import theme from './theme';
+import { Switch, Route, Redirect, useHistory } from 'react-router-dom';
 import Layout from './components/Layout';
 import RouterData from './router';
 import PinotMethodUtils from './utils/PinotMethodUtils';
-import CustomNotification from './components/CustomNotification';
-import { NotificationContextProvider } from './components/Notification/NotificationContextProvider';
 import app_state from './app_state';
+import { useAuthProvider } from './components/auth/AuthProvider';
+import { AppLoadingIndicator } from './components/AppLoadingIndicator';
 import { AuthWorkflow } from 'Models';
 
-const useStyles = makeStyles(() =>
-  createStyles({
-    loader: {
-      position: 'fixed',
-      left: '50%',
-      top: '30%'
-    },
-  })
-);
-
-const App = () => {
+export const App = () => {
   const [clusterName, setClusterName] = React.useState('');
   const [loading, setLoading] = React.useState(true);
-  const oidcSignInFormRef = React.useRef<HTMLFormElement>(null);
   const [isAuthenticated, setIsAuthenticated] = React.useState(null);
-  const [issuer, setIssuer] = React.useState(null);
-  const [redirectUri, setRedirectUri] = React.useState(null);
-  const [clientId, setClientId] = React.useState(null);
-  const [authWorkflow, setAuthWorkflow] = React.useState(null);
-  const [authorizationEndpoint, setAuthorizationEndpoint] = React.useState(
-    null
-  );
   const [role, setRole] = React.useState('');
+  const { authenticated, authWorkflow } = useAuthProvider();
+  const history = useHistory();
+
+  React.useEffect(() => {

Review Comment:
   fixed.



-- 
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 pull request #10389: feat(ui): persist oidc auth details

Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on PR #10389:
URL: https://github.com/apache/pinot/pull/10389#issuecomment-1462617118

   cc @joshigaurava @apucher 


-- 
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 merged pull request #10389: feat(ui): persist oidc auth details

Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang merged PR #10389:
URL: https://github.com/apache/pinot/pull/10389


-- 
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] codecov-commenter commented on pull request #10389: feat(ui): persist auth details

Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #10389:
URL: https://github.com/apache/pinot/pull/10389#issuecomment-1461872284

   # [Codecov](https://codecov.io/gh/apache/pinot/pull/10389?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#10389](https://codecov.io/gh/apache/pinot/pull/10389?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (bad0e3d) into [master](https://codecov.io/gh/apache/pinot/commit/c5dd6df27c384b523c77adcb66956bf0e5c37251?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c5dd6df) will **increase** coverage by `9.41%`.
   > The diff coverage is `n/a`.
   
   ```diff
   @@             Coverage Diff              @@
   ##             master   #10389      +/-   ##
   ============================================
   + Coverage     60.86%   70.27%   +9.41%     
   - Complexity     5007     6027    +1020     
   ============================================
     Files          2020     2049      +29     
     Lines        109752   111033    +1281     
     Branches      16693    16891     +198     
   ============================================
   + Hits          66804    78033   +11229     
   + Misses        37925    27530   -10395     
   - Partials       5023     5470     +447     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | integration1 | `24.41% <ø> (-0.10%)` | :arrow_down: |
   | integration2 | `24.36% <ø> (?)` | |
   | unittests1 | `67.87% <ø> (+0.16%)` | :arrow_up: |
   | unittests2 | `13.90% <ø> (?)` | |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/pinot/pull/10389?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...g/apache/pinot/server/api/resources/ErrorInfo.java](https://codecov.io/gh/apache/pinot/pull/10389?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zZXJ2ZXIvYXBpL3Jlc291cmNlcy9FcnJvckluZm8uamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...t/server/api/resources/DefaultExceptionMapper.java](https://codecov.io/gh/apache/pinot/pull/10389?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zZXJ2ZXIvYXBpL3Jlc291cmNlcy9EZWZhdWx0RXhjZXB0aW9uTWFwcGVyLmphdmE=) | `0.00% <0.00%> (-75.00%)` | :arrow_down: |
   | [...n/java/org/apache/pinot/query/type/TypeSystem.java](https://codecov.io/gh/apache/pinot/pull/10389?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtcXVlcnktcGxhbm5lci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcXVlcnkvdHlwZS9UeXBlU3lzdGVtLmphdmE=) | `75.00% <0.00%> (-25.00%)` | :arrow_down: |
   | [...re/operator/dociditerators/EmptyDocIdIterator.java](https://codecov.io/gh/apache/pinot/pull/10389?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9kb2NpZGl0ZXJhdG9ycy9FbXB0eURvY0lkSXRlcmF0b3IuamF2YQ==) | `75.00% <0.00%> (-25.00%)` | :arrow_down: |
   | [...apache/pinot/segment/local/upsert/UpsertUtils.java](https://codecov.io/gh/apache/pinot/pull/10389?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC91cHNlcnQvVXBzZXJ0VXRpbHMuamF2YQ==) | `24.24% <0.00%> (-15.76%)` | :arrow_down: |
   | [...ore/query/scheduler/resources/ResourceManager.java](https://codecov.io/gh/apache/pinot/pull/10389?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9zY2hlZHVsZXIvcmVzb3VyY2VzL1Jlc291cmNlTWFuYWdlci5qYXZh) | `88.46% <0.00%> (-11.54%)` | :arrow_down: |
   | [.../pinot/server/api/resources/TableSizeResource.java](https://codecov.io/gh/apache/pinot/pull/10389?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zZXJ2ZXIvYXBpL3Jlc291cmNlcy9UYWJsZVNpemVSZXNvdXJjZS5qYXZh) | `80.00% <0.00%> (-8.00%)` | :arrow_down: |
   | [...n/java/org/apache/pinot/common/utils/URIUtils.java](https://codecov.io/gh/apache/pinot/pull/10389?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdXRpbHMvVVJJVXRpbHMuamF2YQ==) | `71.87% <0.00%> (-6.25%)` | :arrow_down: |
   | [.../pinot/core/query/scheduler/PriorityScheduler.java](https://codecov.io/gh/apache/pinot/pull/10389?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9zY2hlZHVsZXIvUHJpb3JpdHlTY2hlZHVsZXIuamF2YQ==) | `77.77% <0.00%> (-5.56%)` | :arrow_down: |
   | [...elix/core/periodictask/ControllerPeriodicTask.java](https://codecov.io/gh/apache/pinot/pull/10389?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL3BlcmlvZGljdGFzay9Db250cm9sbGVyUGVyaW9kaWNUYXNrLmphdmE=) | `66.07% <0.00%> (-5.36%)` | :arrow_down: |
   | ... and [506 more](https://codecov.io/gh/apache/pinot/pull/10389?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   :mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   


-- 
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] joshigaurava commented on a diff in pull request #10389: feat(ui): persist oidc auth details

Posted by "joshigaurava (via GitHub)" <gi...@apache.org>.
joshigaurava commented on code in PR #10389:
URL: https://github.com/apache/pinot/pull/10389#discussion_r1134040833


##########
pinot-controller/src/main/resources/app/App.tsx:
##########
@@ -18,48 +18,42 @@
  */
 
 import * as React from 'react';
-import * as ReactDOM from 'react-dom';
-import { CircularProgress, createStyles, makeStyles, MuiThemeProvider } from '@material-ui/core';
-import { Switch, Route, HashRouter as Router, Redirect } from 'react-router-dom';
-import theme from './theme';
+import { Switch, Route, Redirect, useHistory } from 'react-router-dom';
 import Layout from './components/Layout';
 import RouterData from './router';
 import PinotMethodUtils from './utils/PinotMethodUtils';
-import CustomNotification from './components/CustomNotification';
-import { NotificationContextProvider } from './components/Notification/NotificationContextProvider';
 import app_state from './app_state';
+import { useAuthProvider } from './components/auth/AuthProvider';
+import { AppLoadingIndicator } from './components/AppLoadingIndicator';
 import { AuthWorkflow } from 'Models';
 
-const useStyles = makeStyles(() =>
-  createStyles({
-    loader: {
-      position: 'fixed',
-      left: '50%',
-      top: '30%'
-    },
-  })
-);
-
-const App = () => {
+export const App = () => {
   const [clusterName, setClusterName] = React.useState('');
   const [loading, setLoading] = React.useState(true);
-  const oidcSignInFormRef = React.useRef<HTMLFormElement>(null);
   const [isAuthenticated, setIsAuthenticated] = React.useState(null);
-  const [issuer, setIssuer] = React.useState(null);
-  const [redirectUri, setRedirectUri] = React.useState(null);
-  const [clientId, setClientId] = React.useState(null);
-  const [authWorkflow, setAuthWorkflow] = React.useState(null);
-  const [authorizationEndpoint, setAuthorizationEndpoint] = React.useState(
-    null
-  );
   const [role, setRole] = React.useState('');
+  const { authenticated, authWorkflow } = useAuthProvider();
+  const history = useHistory();
+
+  React.useEffect(() => {

Review Comment:
   Minor and not a blocker: can you pleease fix the import so that you can use just `useEffect`?



-- 
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] joshigaurava commented on pull request #10389: feat(ui): persist oidc auth details

Posted by "joshigaurava (via GitHub)" <gi...@apache.org>.
joshigaurava commented on PR #10389:
URL: https://github.com/apache/pinot/pull/10389#issuecomment-1466315635

   Looks good.


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