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 2021/03/10 18:17:48 UTC

[GitHub] [incubator-pinot] shahsank3t opened a new pull request #6665: Query console only view for Cluster Manager

shahsank3t opened a new pull request #6665:
URL: https://github.com/apache/incubator-pinot/pull/6665


   ## Description
   This PR is for https://github.com/apache/incubator-pinot/issues/6619
   
   Added a configuration file: `pinot-controller/src/main/resources/app/config.json` which will store the flag `showOnlyQueryConsole` (default to false)
   
   If the flag is set to `true`, UI will only display Query Console and all the other routes will redirect to the same Query Console page as visible below.
   ![image](https://user-images.githubusercontent.com/6761317/110676677-3b33f500-81fa-11eb-806c-25a292365adc.png)
   


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



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


[GitHub] [incubator-pinot] npawar commented on pull request #6665: Query console only view for Cluster Manager

Posted by GitBox <gi...@apache.org>.
npawar commented on pull request #6665:
URL: https://github.com/apache/incubator-pinot/pull/6665#issuecomment-800546727


   @icefury71 not part of this PR but related. Do we also want to add the "queryConsoleViewOnly" property to HelixSetupUtils with default false? I'm thinking not from the perspective of default value for UI, but more for how will anyone discover this property exists, unless they look at UI code. If the default prop is added, it will show up in the cluster configs.


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



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


[GitHub] [incubator-pinot] joshigaurava commented on a change in pull request #6665: Query console only view for Cluster Manager

Posted by GitBox <gi...@apache.org>.
joshigaurava commented on a change in pull request #6665:
URL: https://github.com/apache/incubator-pinot/pull/6665#discussion_r595453938



##########
File path: pinot-controller/src/main/resources/app/App.tsx
##########
@@ -19,49 +19,87 @@
 
 import * as React from 'react';
 import * as ReactDOM from 'react-dom';
-import { MuiThemeProvider } from '@material-ui/core';
-import { Switch, Route, HashRouter as Router } from 'react-router-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 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';
+
+const useStyles = makeStyles(() =>
+  createStyles({
+    loader: {
+      position: 'fixed',
+      left: '50%',
+      top: '30%'
+    },
+  })
+);
 
 const App = () => {
   const [clusterName, setClusterName] = React.useState('');
+  const [loading, setLoading] = React.useState(true);
+
   const fetchClusterName = async () => {
     const clusterNameResponse = await PinotMethodUtils.getClusterName();
     localStorage.setItem('pinot_ui:clusterName', clusterNameResponse);
     setClusterName(clusterNameResponse);
   };
+
+  const fetchClusterConfig = async () => {
+    const clusterConfig = await PinotMethodUtils.getClusterConfigJSON();
+    app_state.queryConsoleOnlyView = clusterConfig?.queryConsoleOnlyView === 'true';

Review comment:
       `app_state.queryConsoleOnlyView` will remain `undefined`/`false` if either of `clusterConfig` or `clusterConfig.queryConsoleOnlyView` is undefined, or if `queryConsoleOnlyView` is explicitly set to "false".




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



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


[GitHub] [incubator-pinot] icefury71 commented on a change in pull request #6665: Query console only view for Cluster Manager

Posted by GitBox <gi...@apache.org>.
icefury71 commented on a change in pull request #6665:
URL: https://github.com/apache/incubator-pinot/pull/6665#discussion_r595442682



##########
File path: pinot-controller/src/main/resources/app/App.tsx
##########
@@ -19,49 +19,87 @@
 
 import * as React from 'react';
 import * as ReactDOM from 'react-dom';
-import { MuiThemeProvider } from '@material-ui/core';
-import { Switch, Route, HashRouter as Router } from 'react-router-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 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';
+
+const useStyles = makeStyles(() =>
+  createStyles({
+    loader: {
+      position: 'fixed',
+      left: '50%',
+      top: '30%'
+    },
+  })
+);
 
 const App = () => {
   const [clusterName, setClusterName] = React.useState('');
+  const [loading, setLoading] = React.useState(true);
+
   const fetchClusterName = async () => {
     const clusterNameResponse = await PinotMethodUtils.getClusterName();
     localStorage.setItem('pinot_ui:clusterName', clusterNameResponse);
     setClusterName(clusterNameResponse);
   };
+
+  const fetchClusterConfig = async () => {
+    const clusterConfig = await PinotMethodUtils.getClusterConfigJSON();
+    app_state.queryConsoleOnlyView = clusterConfig?.queryConsoleOnlyView === 'true';

Review comment:
       Apologies for my lack of syntax understanding, but will this set the `app_state.queryConsoleOnlyView` flag to false if queryConsoleOnlyView is missing or is set to false in the config ?




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



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


[GitHub] [incubator-pinot] shahsank3t commented on a change in pull request #6665: Query console only view for Cluster Manager

Posted by GitBox <gi...@apache.org>.
shahsank3t commented on a change in pull request #6665:
URL: https://github.com/apache/incubator-pinot/pull/6665#discussion_r595453247



##########
File path: pinot-controller/src/main/resources/app/App.tsx
##########
@@ -19,49 +19,87 @@
 
 import * as React from 'react';
 import * as ReactDOM from 'react-dom';
-import { MuiThemeProvider } from '@material-ui/core';
-import { Switch, Route, HashRouter as Router } from 'react-router-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 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';
+
+const useStyles = makeStyles(() =>
+  createStyles({
+    loader: {
+      position: 'fixed',
+      left: '50%',
+      top: '30%'
+    },
+  })
+);
 
 const App = () => {
   const [clusterName, setClusterName] = React.useState('');
+  const [loading, setLoading] = React.useState(true);
+
   const fetchClusterName = async () => {
     const clusterNameResponse = await PinotMethodUtils.getClusterName();
     localStorage.setItem('pinot_ui:clusterName', clusterNameResponse);
     setClusterName(clusterNameResponse);
   };
+
+  const fetchClusterConfig = async () => {
+    const clusterConfig = await PinotMethodUtils.getClusterConfigJSON();
+    app_state.queryConsoleOnlyView = clusterConfig?.queryConsoleOnlyView === 'true';

Review comment:
       yes, that right. only if there is queryConsoleOnlyView and set as  "true" in string then it will be set to true else false.




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



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


[GitHub] [incubator-pinot] icefury71 merged pull request #6665: Query console only view for Cluster Manager

Posted by GitBox <gi...@apache.org>.
icefury71 merged pull request #6665:
URL: https://github.com/apache/incubator-pinot/pull/6665


   


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



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


[GitHub] [incubator-pinot] icefury71 commented on a change in pull request #6665: Query console only view for Cluster Manager

Posted by GitBox <gi...@apache.org>.
icefury71 commented on a change in pull request #6665:
URL: https://github.com/apache/incubator-pinot/pull/6665#discussion_r595459594



##########
File path: pinot-controller/src/main/resources/app/App.tsx
##########
@@ -19,49 +19,87 @@
 
 import * as React from 'react';
 import * as ReactDOM from 'react-dom';
-import { MuiThemeProvider } from '@material-ui/core';
-import { Switch, Route, HashRouter as Router } from 'react-router-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 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';
+
+const useStyles = makeStyles(() =>
+  createStyles({
+    loader: {
+      position: 'fixed',
+      left: '50%',
+      top: '30%'
+    },
+  })
+);
 
 const App = () => {
   const [clusterName, setClusterName] = React.useState('');
+  const [loading, setLoading] = React.useState(true);
+
   const fetchClusterName = async () => {
     const clusterNameResponse = await PinotMethodUtils.getClusterName();
     localStorage.setItem('pinot_ui:clusterName', clusterNameResponse);
     setClusterName(clusterNameResponse);
   };
+
+  const fetchClusterConfig = async () => {
+    const clusterConfig = await PinotMethodUtils.getClusterConfigJSON();
+    app_state.queryConsoleOnlyView = clusterConfig?.queryConsoleOnlyView === 'true';

Review comment:
       ok sounds good. That was the expectation. Thanks for confirming.




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



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


[GitHub] [incubator-pinot] icefury71 commented on pull request #6665: Query console only view for Cluster Manager

Posted by GitBox <gi...@apache.org>.
icefury71 commented on pull request #6665:
URL: https://github.com/apache/incubator-pinot/pull/6665#issuecomment-800583293


   > @icefury71 not part of this PR but related. Do we also want to add the "queryConsoleViewOnly" property to HelixSetupUtils with default false? I'm thinking not from the perspective of default value for UI, but more for how will anyone discover this property exists, unless they look at UI code. If the default prop is added, it will show up in the cluster configs.
   
   Yep - will open a simple PR for that


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



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