You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by GitBox <gi...@apache.org> on 2020/06/11 09:40:57 UTC
[GitHub] [incubator-apisix-dashboard] LiteSun opened a new pull request #255: add: metrics(Grafana)
LiteSun opened a new pull request #255:
URL: https://github.com/apache/incubator-apisix-dashboard/pull/255
----------------------------------------------------------------
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
[GitHub] [incubator-apisix-dashboard] juzhiyuan commented on a change in pull request #255: add: metrics(Grafana)
Posted by GitBox <gi...@apache.org>.
juzhiyuan commented on a change in pull request #255:
URL: https://github.com/apache/incubator-apisix-dashboard/pull/255#discussion_r438834571
##########
File path: src/pages/Metrics/Metrics.tsx
##########
@@ -1,17 +1,24 @@
-import React, { useState } from 'react';
+import React, { useState, useEffect } from 'react';
import { PageHeaderWrapper } from '@ant-design/pro-layout';
import { Empty, Button, Card } from 'antd';
import { history } from 'umi';
import { stringify } from 'qs';
+import { getGrafanaConfig } from './service';
const Metrics: React.FC<{}> = () => {
- const GLOBAL_ADMIN_SETTING_GRAFANA_URL =
- localStorage.getItem('GLOBAL_ADMIN_SETTING_GRAFANA_URL') || undefined;
- const [showMetricsDashboard] = useState(GLOBAL_ADMIN_SETTING_GRAFANA_URL);
+ const [grafanaURL, setGrafanaURL] = useState<string>('');
+ const [showMetrics, setShowMetrics] = useState(Boolean(grafanaURL));
+
+ useEffect(() => {
+ const url = getGrafanaConfig();
Review comment:
getXXConfig should return an Object but not a fixed value...
----------------------------------------------------------------
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
[GitHub] [incubator-apisix-dashboard] juzhiyuan merged pull request #255: add: metrics(Grafana)
Posted by GitBox <gi...@apache.org>.
juzhiyuan merged pull request #255:
URL: https://github.com/apache/incubator-apisix-dashboard/pull/255
----------------------------------------------------------------
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
[GitHub] [incubator-apisix-dashboard] LiteSun commented on a change in pull request #255: add: metrics(Grafana)
Posted by GitBox <gi...@apache.org>.
LiteSun commented on a change in pull request #255:
URL: https://github.com/apache/incubator-apisix-dashboard/pull/255#discussion_r438846378
##########
File path: src/pages/Setting/Setting.tsx
##########
@@ -14,15 +14,21 @@ const Settings: React.FC<{}> = () => {
const { formatMessage } = useIntl();
useEffect(() => {
- form.setFieldsValue(getAdminAPIConfig());
+ form.setFieldsValue({ ...getAdminAPIConfig(), ...getGrafanaConfig() });
Review comment:
?
----------------------------------------------------------------
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
[GitHub] [incubator-apisix-dashboard] juzhiyuan commented on a change in pull request #255: add: metrics(Grafana)
Posted by GitBox <gi...@apache.org>.
juzhiyuan commented on a change in pull request #255:
URL: https://github.com/apache/incubator-apisix-dashboard/pull/255#discussion_r439085210
##########
File path: src/pages/Metrics/Metrics.tsx
##########
@@ -0,0 +1,53 @@
+import React, { useState } from 'react';
+import { PageHeaderWrapper } from '@ant-design/pro-layout';
+import { Empty, Button, Card } from 'antd';
+import { history } from 'umi';
+import { stringify } from 'qs';
+
+const Metrics: React.FC<{}> = () => {
Review comment:
yep but not in this PR, those files were generated by Ant Design Pro.
----------------------------------------------------------------
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
[GitHub] [incubator-apisix-dashboard] juzhiyuan commented on a change in pull request #255: add: metrics(Grafana)
Posted by GitBox <gi...@apache.org>.
juzhiyuan commented on a change in pull request #255:
URL: https://github.com/apache/incubator-apisix-dashboard/pull/255#discussion_r438834127
##########
File path: src/pages/Metrics/Metrics.tsx
##########
@@ -1,17 +1,24 @@
-import React, { useState } from 'react';
+import React, { useState, useEffect } from 'react';
import { PageHeaderWrapper } from '@ant-design/pro-layout';
import { Empty, Button, Card } from 'antd';
import { history } from 'umi';
import { stringify } from 'qs';
+import { getGrafanaConfig } from './service';
const Metrics: React.FC<{}> = () => {
- const GLOBAL_ADMIN_SETTING_GRAFANA_URL =
- localStorage.getItem('GLOBAL_ADMIN_SETTING_GRAFANA_URL') || undefined;
- const [showMetricsDashboard] = useState(GLOBAL_ADMIN_SETTING_GRAFANA_URL);
+ const [grafanaURL, setGrafanaURL] = useState<string>('');
Review comment:
const [] = useState<string | undefined>()
----------------------------------------------------------------
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
[GitHub] [incubator-apisix-dashboard] LiteSun commented on a change in pull request #255: add: metrics(Grafana)
Posted by GitBox <gi...@apache.org>.
LiteSun commented on a change in pull request #255:
URL: https://github.com/apache/incubator-apisix-dashboard/pull/255#discussion_r438866263
##########
File path: src/pages/Metrics/Metrics.tsx
##########
@@ -0,0 +1,53 @@
+import React, { useState } from 'react';
+import { PageHeaderWrapper } from '@ant-design/pro-layout';
+import { Empty, Button, Card } from 'antd';
+import { history } from 'umi';
+import { stringify } from 'qs';
+
+const Metrics: React.FC<{}> = () => {
Review comment:
should I remove `React.FC<{}>` in `pages/404.tsx` `pages/Setting/Setting.tsx` file?
----------------------------------------------------------------
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
[GitHub] [incubator-apisix-dashboard] juzhiyuan commented on a change in pull request #255: add: metrics(Grafana)
Posted by GitBox <gi...@apache.org>.
juzhiyuan commented on a change in pull request #255:
URL: https://github.com/apache/incubator-apisix-dashboard/pull/255#discussion_r438836932
##########
File path: src/pages/Metrics/Metrics.tsx
##########
@@ -0,0 +1,53 @@
+import React, { useState } from 'react';
+import { PageHeaderWrapper } from '@ant-design/pro-layout';
+import { Empty, Button, Card } from 'antd';
+import { history } from 'umi';
+import { stringify } from 'qs';
+
+const Metrics: React.FC<{}> = () => {
Review comment:
empty typing should be remove
----------------------------------------------------------------
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
[GitHub] [incubator-apisix-dashboard] juzhiyuan commented on a change in pull request #255: add: metrics(Grafana)
Posted by GitBox <gi...@apache.org>.
juzhiyuan commented on a change in pull request #255:
URL: https://github.com/apache/incubator-apisix-dashboard/pull/255#discussion_r438835257
##########
File path: src/pages/Metrics/Metrics.tsx
##########
@@ -1,17 +1,24 @@
-import React, { useState } from 'react';
+import React, { useState, useEffect } from 'react';
import { PageHeaderWrapper } from '@ant-design/pro-layout';
import { Empty, Button, Card } from 'antd';
import { history } from 'umi';
import { stringify } from 'qs';
+import { getGrafanaConfig } from './service';
const Metrics: React.FC<{}> = () => {
- const GLOBAL_ADMIN_SETTING_GRAFANA_URL =
- localStorage.getItem('GLOBAL_ADMIN_SETTING_GRAFANA_URL') || undefined;
- const [showMetricsDashboard] = useState(GLOBAL_ADMIN_SETTING_GRAFANA_URL);
+ const [grafanaURL, setGrafanaURL] = useState<string>('');
+ const [showMetrics, setShowMetrics] = useState(Boolean(grafanaURL));
Review comment:
always 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
[GitHub] [incubator-apisix-dashboard] LiteSun commented on a change in pull request #255: add: metrics(Grafana)
Posted by GitBox <gi...@apache.org>.
LiteSun commented on a change in pull request #255:
URL: https://github.com/apache/incubator-apisix-dashboard/pull/255#discussion_r438846707
##########
File path: src/pages/Setting/typingd.d.ts
##########
@@ -6,7 +6,7 @@ declare namespace Setting {
key: string;
}
- interface DashboardConfig extends AdminAPI {
- grafanaUrl: string;
+ interface GrafanaURL {
Review comment:
?
----------------------------------------------------------------
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
[GitHub] [incubator-apisix-dashboard] juzhiyuan commented on a change in pull request #255: add: metrics(Grafana)
Posted by GitBox <gi...@apache.org>.
juzhiyuan commented on a change in pull request #255:
URL: https://github.com/apache/incubator-apisix-dashboard/pull/255#discussion_r439085446
##########
File path: src/pages/Setting/typingd.d.ts
##########
@@ -6,7 +6,7 @@ declare namespace Setting {
key: string;
}
- interface DashboardConfig extends AdminAPI {
- grafanaUrl: string;
+ interface GrafanaURL {
Review comment:
Why the typing GrafanaURL includes a grafanaURL property??
----------------------------------------------------------------
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
[GitHub] [incubator-apisix-dashboard] juzhiyuan commented on a change in pull request #255: add: metrics(Grafana)
Posted by GitBox <gi...@apache.org>.
juzhiyuan commented on a change in pull request #255:
URL: https://github.com/apache/incubator-apisix-dashboard/pull/255#discussion_r438666207
##########
File path: src/pages/Metrics/Metrics.tsx
##########
@@ -0,0 +1,53 @@
+import React, { useState } from 'react';
+import { PageHeaderWrapper } from '@ant-design/pro-layout';
+import { Empty, Button, Card } from 'antd';
+import { history } from 'umi';
+import { stringify } from 'qs';
+
+const Metrics: React.FC<{}> = () => {
Review comment:
,
##########
File path: src/pages/Metrics/Metrics.tsx
##########
@@ -0,0 +1,53 @@
+import React, { useState } from 'react';
+import { PageHeaderWrapper } from '@ant-design/pro-layout';
+import { Empty, Button, Card } from 'antd';
+import { history } from 'umi';
+import { stringify } from 'qs';
+
+const Metrics: React.FC<{}> = () => {
+ const GLOBAL_ADMIN_SETTING_GRAFANA_URL =
+ localStorage.getItem('GLOBAL_ADMIN_SETTING_GRAFANA_URL') || undefined;
+ const [showMetricsDashboard] = useState(GLOBAL_ADMIN_SETTING_GRAFANA_URL);
Review comment:
or `const grafanaURL = xxx`
##########
File path: src/pages/Setting/service.ts
##########
@@ -1,8 +1,9 @@
-export const getAdminAPIConfig = (): Setting.AdminAPI => {
+export const getAdminAPIConfig = (): Setting.DashboardConfig => {
Review comment:
A & B
##########
File path: src/pages/Metrics/Metrics.tsx
##########
@@ -0,0 +1,53 @@
+import React, { useState } from 'react';
+import { PageHeaderWrapper } from '@ant-design/pro-layout';
+import { Empty, Button, Card } from 'antd';
+import { history } from 'umi';
+import { stringify } from 'qs';
+
+const Metrics: React.FC<{}> = () => {
+ const GLOBAL_ADMIN_SETTING_GRAFANA_URL =
+ localStorage.getItem('GLOBAL_ADMIN_SETTING_GRAFANA_URL') || undefined;
Review comment:
undefined?
##########
File path: src/pages/Metrics/Metrics.tsx
##########
@@ -0,0 +1,53 @@
+import React, { useState } from 'react';
+import { PageHeaderWrapper } from '@ant-design/pro-layout';
+import { Empty, Button, Card } from 'antd';
+import { history } from 'umi';
+import { stringify } from 'qs';
+
+const Metrics: React.FC<{}> = () => {
+ const GLOBAL_ADMIN_SETTING_GRAFANA_URL =
+ localStorage.getItem('GLOBAL_ADMIN_SETTING_GRAFANA_URL') || undefined;
+ const [showMetricsDashboard] = useState(GLOBAL_ADMIN_SETTING_GRAFANA_URL);
Review comment:
1. showMetrics is fine;
2. useState(Boolean(localStorage.getItem('GLOBAL_ADMIN_SETTING_GRAFANA_URL')))
##########
File path: src/pages/Setting/typingd.d.ts
##########
@@ -5,4 +5,8 @@ declare namespace Setting {
path: string;
key: string;
}
+
+ interface DashboardConfig extends AdminAPI {
Review comment:
,
##########
File path: src/pages/Setting/service.ts
##########
@@ -1,8 +1,9 @@
-export const getAdminAPIConfig = (): Setting.AdminAPI => {
+export const getAdminAPIConfig = (): Setting.DashboardConfig => {
return {
schema: localStorage.getItem('GLOBAL_ADMIN_API_SCHEMA') || 'http',
host: localStorage.getItem('GLOBAL_ADMIN_API_HOST') || '127.0.0.1:9080',
path: localStorage.getItem('GLOBAL_ADMIN_API_PATH') || '/apisix/admin',
key: localStorage.getItem('GLOBAL_ADMIN_API_KEY') || '',
+ grafanaUrl: localStorage.getItem('GLOBAL_ADMIN_SETTING_GRAFANA_URL') || '',
Review comment:
grafanaURL
##########
File path: src/pages/Metrics/Metrics.tsx
##########
@@ -0,0 +1,53 @@
+import React, { useState } from 'react';
+import { PageHeaderWrapper } from '@ant-design/pro-layout';
+import { Empty, Button, Card } from 'antd';
+import { history } from 'umi';
+import { stringify } from 'qs';
+
+const Metrics: React.FC<{}> = () => {
+ const GLOBAL_ADMIN_SETTING_GRAFANA_URL =
+ localStorage.getItem('GLOBAL_ADMIN_SETTING_GRAFANA_URL') || undefined;
Review comment:
from service?
----------------------------------------------------------------
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
[GitHub] [incubator-apisix-dashboard] juzhiyuan commented on a change in pull request #255: add: metrics(Grafana)
Posted by GitBox <gi...@apache.org>.
juzhiyuan commented on a change in pull request #255:
URL: https://github.com/apache/incubator-apisix-dashboard/pull/255#discussion_r438835450
##########
File path: src/pages/Metrics/service.ts
##########
@@ -0,0 +1,3 @@
+export const getGrafanaConfig = (): string => {
+ return localStorage.getItem('GLOBAL_ADMIN_SETTING_GRAFANA_URL') || '';
Review comment:
,
##########
File path: src/pages/Setting/typingd.d.ts
##########
@@ -6,7 +6,7 @@ declare namespace Setting {
key: string;
}
- interface DashboardConfig extends AdminAPI {
- grafanaUrl: string;
+ interface GrafanaURL {
Review comment:
too wired...
##########
File path: src/pages/Setting/Setting.tsx
##########
@@ -14,15 +14,21 @@ const Settings: React.FC<{}> = () => {
const { formatMessage } = useIntl();
useEffect(() => {
- form.setFieldsValue(getAdminAPIConfig());
+ form.setFieldsValue({ ...getAdminAPIConfig(), ...getGrafanaConfig() });
Review comment:
,
----------------------------------------------------------------
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
[GitHub] [incubator-apisix-dashboard] juzhiyuan commented on a change in pull request #255: add: metrics(Grafana)
Posted by GitBox <gi...@apache.org>.
juzhiyuan commented on a change in pull request #255:
URL: https://github.com/apache/incubator-apisix-dashboard/pull/255#discussion_r439085707
##########
File path: src/pages/Setting/typingd.d.ts
##########
@@ -6,7 +6,7 @@ declare namespace Setting {
key: string;
}
- interface GrafanaURL {
+ interface GrafanaConfig {
grafanaURL: string;
Review comment:
that's it..
----------------------------------------------------------------
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