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/09/19 16:47:12 UTC

[GitHub] [apisix-dashboard] liuxiran opened a new pull request #489: feat: Upstream health check

liuxiran opened a new pull request #489:
URL: https://github.com/apache/apisix-dashboard/pull/489


   Please answer these questions before submitting a pull request
   
   - Why submit this pull request?
   - [ ] Bug fix
   - [ ] New feature provided
   - [ ] Improve performance
   
   - Related issues
   fix #470 
   ___
   ### Bugfix
   - Description
   
   - How to fix?
   
   ___
   ### New feature or improvement
   - Describe the details and related test reports.
   


----------------------------------------------------------------
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] [apisix-dashboard] juzhiyuan commented on a change in pull request #489: feat: Upstream health check

Posted by GitBox <gi...@apache.org>.
juzhiyuan commented on a change in pull request #489:
URL: https://github.com/apache/apisix-dashboard/pull/489#discussion_r491769342



##########
File path: src/pages/Upstream/components/Step1.tsx
##########
@@ -43,9 +47,38 @@ const initialValues = {
     send: 6000,
     read: 6000,
   },
+  active: false,
+  passive: false,
+  checks: {
+    active: {
+      timeout: 5,
+      http_path: '',
+      host: '',
+      healthy: {
+        interval: 2,
+        successes: 1,
+      },
+      unhealthy: {
+        interval: 1,
+        http_failures: 2,
+      },
+      req_headers: [''],
+    },
+    passive: {
+      healthy: {
+        http_statuses: [undefined],

Review comment:
       could we use an empty array here?

##########
File path: src/pages/Upstream/components/Step1.tsx
##########
@@ -171,6 +204,390 @@ const Step1: React.FC<Props> = ({ form, disabled }) => {
 
   const renderTimeUnit = () => <span style={{ margin: '0 8px' }}>ms</span>;
 
+  function activeChange() {

Review comment:
       Also, for onChange handler, I would prefer using some name like `handleXXX`

##########
File path: src/pages/Upstream/components/Step1.tsx
##########
@@ -171,6 +204,390 @@ const Step1: React.FC<Props> = ({ form, disabled }) => {
 
   const renderTimeUnit = () => <span style={{ margin: '0 8px' }}>ms</span>;
 
+  function activeChange() {

Review comment:
       We'd better use `const fn = () => {}` to have a unified style :D

##########
File path: src/pages/Upstream/components/Step1.tsx
##########
@@ -171,6 +204,390 @@ const Step1: React.FC<Props> = ({ form, disabled }) => {
 
   const renderTimeUnit = () => <span style={{ margin: '0 8px' }}>ms</span>;
 
+  function activeChange() {
+    if (isActive) {
+      onChange(!isActive, false);
+      form.setFieldsValue({ ...form.getFieldsValue(), passive: false });
+      return;
+    }
+    onChange(!isActive, isPassive);
+    form.setFieldsValue({ ...form.getFieldsValue(), active: !isActive });
+  }
+  function passiveChange() {

Review comment:
       same here




----------------------------------------------------------------
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] [apisix-dashboard] juzhiyuan commented on a change in pull request #489: feat: Upstream health check

Posted by GitBox <gi...@apache.org>.
juzhiyuan commented on a change in pull request #489:
URL: https://github.com/apache/apisix-dashboard/pull/489#discussion_r492094546



##########
File path: src/pages/Upstream/components/Step1.tsx
##########
@@ -43,9 +47,38 @@ const initialValues = {
     send: 6000,
     read: 6000,
   },
+  active: false,
+  passive: false,
+  checks: {
+    active: {
+      timeout: 5,
+      http_path: '',
+      host: '',
+      healthy: {
+        interval: 2,
+        successes: 1,
+      },
+      unhealthy: {
+        interval: 1,
+        http_failures: 2,
+      },
+      req_headers: [''],
+    },
+    passive: {
+      healthy: {
+        http_statuses: [undefined],

Review comment:
       got it, thanks!




----------------------------------------------------------------
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] [apisix-dashboard] juzhiyuan commented on a change in pull request #489: feat: Upstream health check

Posted by GitBox <gi...@apache.org>.
juzhiyuan commented on a change in pull request #489:
URL: https://github.com/apache/apisix-dashboard/pull/489#discussion_r491769342



##########
File path: src/pages/Upstream/components/Step1.tsx
##########
@@ -43,9 +47,38 @@ const initialValues = {
     send: 6000,
     read: 6000,
   },
+  active: false,
+  passive: false,
+  checks: {
+    active: {
+      timeout: 5,
+      http_path: '',
+      host: '',
+      healthy: {
+        interval: 2,
+        successes: 1,
+      },
+      unhealthy: {
+        interval: 1,
+        http_failures: 2,
+      },
+      req_headers: [''],
+    },
+    passive: {
+      healthy: {
+        http_statuses: [undefined],

Review comment:
       could we use an empty array here?

##########
File path: src/pages/Upstream/components/Step1.tsx
##########
@@ -171,6 +204,390 @@ const Step1: React.FC<Props> = ({ form, disabled }) => {
 
   const renderTimeUnit = () => <span style={{ margin: '0 8px' }}>ms</span>;
 
+  function activeChange() {

Review comment:
       Also, for onChange handler, I would prefer using some name like `handleXXX`

##########
File path: src/pages/Upstream/components/Step1.tsx
##########
@@ -171,6 +204,390 @@ const Step1: React.FC<Props> = ({ form, disabled }) => {
 
   const renderTimeUnit = () => <span style={{ margin: '0 8px' }}>ms</span>;
 
+  function activeChange() {

Review comment:
       We'd better use `const fn = () => {}` to have a unified style :D

##########
File path: src/pages/Upstream/components/Step1.tsx
##########
@@ -171,6 +204,390 @@ const Step1: React.FC<Props> = ({ form, disabled }) => {
 
   const renderTimeUnit = () => <span style={{ margin: '0 8px' }}>ms</span>;
 
+  function activeChange() {
+    if (isActive) {
+      onChange(!isActive, false);
+      form.setFieldsValue({ ...form.getFieldsValue(), passive: false });
+      return;
+    }
+    onChange(!isActive, isPassive);
+    form.setFieldsValue({ ...form.getFieldsValue(), active: !isActive });
+  }
+  function passiveChange() {

Review comment:
       same here

##########
File path: src/pages/Upstream/components/Step1.tsx
##########
@@ -43,9 +47,38 @@ const initialValues = {
     send: 6000,
     read: 6000,
   },
+  active: false,
+  passive: false,
+  checks: {
+    active: {
+      timeout: 5,
+      http_path: '',
+      host: '',
+      healthy: {
+        interval: 2,
+        successes: 1,
+      },
+      unhealthy: {
+        interval: 1,
+        http_failures: 2,
+      },
+      req_headers: [''],
+    },
+    passive: {
+      healthy: {
+        http_statuses: [undefined],

Review comment:
       got it, thanks!




----------------------------------------------------------------
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] [apisix-dashboard] liuxiran commented on a change in pull request #489: feat: Upstream health check

Posted by GitBox <gi...@apache.org>.
liuxiran commented on a change in pull request #489:
URL: https://github.com/apache/apisix-dashboard/pull/489#discussion_r492048661



##########
File path: src/pages/Upstream/components/Step1.tsx
##########
@@ -171,6 +204,390 @@ const Step1: React.FC<Props> = ({ form, disabled }) => {
 
   const renderTimeUnit = () => <span style={{ margin: '0 8px' }}>ms</span>;
 
+  function activeChange() {

Review comment:
       done




----------------------------------------------------------------
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] [apisix-dashboard] liuxiran commented on a change in pull request #489: feat: Upstream health check

Posted by GitBox <gi...@apache.org>.
liuxiran commented on a change in pull request #489:
URL: https://github.com/apache/apisix-dashboard/pull/489#discussion_r492048809



##########
File path: src/pages/Upstream/components/Step1.tsx
##########
@@ -171,6 +204,390 @@ const Step1: React.FC<Props> = ({ form, disabled }) => {
 
   const renderTimeUnit = () => <span style={{ margin: '0 8px' }}>ms</span>;
 
+  function activeChange() {
+    if (isActive) {
+      onChange(!isActive, false);
+      form.setFieldsValue({ ...form.getFieldsValue(), passive: false });
+      return;
+    }
+    onChange(!isActive, isPassive);
+    form.setFieldsValue({ ...form.getFieldsValue(), active: !isActive });
+  }
+  function passiveChange() {

Review comment:
       done




----------------------------------------------------------------
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] [apisix-dashboard] juzhiyuan commented on pull request #489: feat: Upstream health check

Posted by GitBox <gi...@apache.org>.
juzhiyuan commented on pull request #489:
URL: https://github.com/apache/apisix-dashboard/pull/489#issuecomment-695364582


   waiting for https://github.com/apache/apisix-dashboard/issues/470#issuecomment-695333690


----------------------------------------------------------------
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] [apisix-dashboard] juzhiyuan merged pull request #489: feat: Upstream health check

Posted by GitBox <gi...@apache.org>.
juzhiyuan merged pull request #489:
URL: https://github.com/apache/apisix-dashboard/pull/489


   


----------------------------------------------------------------
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] [apisix-dashboard] juzhiyuan merged pull request #489: feat: Upstream health check

Posted by GitBox <gi...@apache.org>.
juzhiyuan merged pull request #489:
URL: https://github.com/apache/apisix-dashboard/pull/489


   


----------------------------------------------------------------
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] [apisix-dashboard] liuxiran commented on a change in pull request #489: feat: Upstream health check

Posted by GitBox <gi...@apache.org>.
liuxiran commented on a change in pull request #489:
URL: https://github.com/apache/apisix-dashboard/pull/489#discussion_r492046907



##########
File path: src/pages/Upstream/components/Step1.tsx
##########
@@ -43,9 +47,38 @@ const initialValues = {
     send: 6000,
     read: 6000,
   },
+  active: false,
+  passive: false,
+  checks: {
+    active: {
+      timeout: 5,
+      http_path: '',
+      host: '',
+      healthy: {
+        interval: 2,
+        successes: 1,
+      },
+      unhealthy: {
+        interval: 1,
+        http_failures: 2,
+      },
+      req_headers: [''],
+    },
+    passive: {
+      healthy: {
+        http_statuses: [undefined],

Review comment:
       empty array will lose  the first item  while rendering Form.List 
   we tried to use [''], but the type of the http_statuses: number[], so we use [undefined] here




----------------------------------------------------------------
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] [apisix-dashboard] liuxiran commented on a change in pull request #489: feat: Upstream health check

Posted by GitBox <gi...@apache.org>.
liuxiran commented on a change in pull request #489:
URL: https://github.com/apache/apisix-dashboard/pull/489#discussion_r492046907



##########
File path: src/pages/Upstream/components/Step1.tsx
##########
@@ -43,9 +47,38 @@ const initialValues = {
     send: 6000,
     read: 6000,
   },
+  active: false,
+  passive: false,
+  checks: {
+    active: {
+      timeout: 5,
+      http_path: '',
+      host: '',
+      healthy: {
+        interval: 2,
+        successes: 1,
+      },
+      unhealthy: {
+        interval: 1,
+        http_failures: 2,
+      },
+      req_headers: [''],
+    },
+    passive: {
+      healthy: {
+        http_statuses: [undefined],

Review comment:
       empty array will lose  the first item  while rendering Form.List 
   we tried to use [''], but the type of the http_statuses: number[], so we use [undefined] here

##########
File path: src/pages/Upstream/components/Step1.tsx
##########
@@ -171,6 +204,390 @@ const Step1: React.FC<Props> = ({ form, disabled }) => {
 
   const renderTimeUnit = () => <span style={{ margin: '0 8px' }}>ms</span>;
 
+  function activeChange() {

Review comment:
       done

##########
File path: src/pages/Upstream/components/Step1.tsx
##########
@@ -171,6 +204,390 @@ const Step1: React.FC<Props> = ({ form, disabled }) => {
 
   const renderTimeUnit = () => <span style={{ margin: '0 8px' }}>ms</span>;
 
+  function activeChange() {
+    if (isActive) {
+      onChange(!isActive, false);
+      form.setFieldsValue({ ...form.getFieldsValue(), passive: false });
+      return;
+    }
+    onChange(!isActive, isPassive);
+    form.setFieldsValue({ ...form.getFieldsValue(), active: !isActive });
+  }
+  function passiveChange() {

Review comment:
       done




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