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 2021/11/23 08:58:40 UTC
[GitHub] [apisix] shuaijinchao opened a new pull request #5589: fix: invalid error after passive health check is changed
shuaijinchao opened a new pull request #5589:
URL: https://github.com/apache/apisix/pull/5589
### What this PR does / why we need it:
FIX #4077
### Pre-submission checklist:
<!--
Please follow the requirements:
1. Use Draft if the PR is not ready to be reviewed
2. Test is required for the feat/fix PR, unless you have a good reason
3. Doc is required for the feat PR
4. Use a new commit to resolve review instead of `push -f`
5. Use "request review" to notify the reviewer once you have resolved the review
-->
* [x] Did you explain what problem does this PR solve? Or what new features have been added?
* [x] Have you added corresponding test cases?
* [ ] Have you modified the corresponding document?
* [x] Is this PR backward compatible? **If it is not backward compatible, please discuss on the [mailing list](https://github.com/apache/apisix/tree/master#community) first**
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a change in pull request #5589: fix: invalid error after passive health check is changed
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #5589:
URL: https://github.com/apache/apisix/pull/5589#discussion_r755622646
##########
File path: t/admin/health-check.t
##########
@@ -469,17 +434,64 @@ GET /t
ngx.print(body)
}
}
---- request
-GET /t
--- error_code: 400
--- response_body
{"error_msg":"invalid configuration: property \"upstream\" validation failed: property \"checks\" validation failed: object matches none of the requireds: [\"active\"] or [\"active\",\"passive\"]"}
---- no_error_log
-[error]
-=== TEST 13: number type timeout
+=== TEST 13: only active
Review comment:
```suggestion
=== TEST 13: change active & passive to only active
```
##########
File path: apisix/schema_def.lua
##########
@@ -259,24 +259,38 @@ local health_checker = {
},
tcp_failures = {
type = "integer",
- minimum = 1,
+ minimum = 0,
maximum = 254,
default = 2
},
timeouts = {
type = "integer",
- minimum = 1,
+ minimum = 0,
maximum = 254,
default = 7
},
http_failures = {
type = "integer",
- minimum = 1,
+ minimum = 0,
maximum = 254,
default = 5
},
}
}
+ },
+ default = {
+ type = "http",
+ healthy = {
+ http_statuses = { 200, 201, 202, 203, 204, 205, 206, 207, 208, 226,
+ 300, 301, 302, 303, 304, 305, 306, 307, 308 },
+ successes = 0,
+ },
+ unhealthy = {
+ http_statuses = { 429, 500, 503 },
+ tcp_failures = 0,
Review comment:
Is it necessary to set a zero default? Do we need to update the doc?
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander merged pull request #5589: fix: invalid error after passive health check is changed
Posted by GitBox <gi...@apache.org>.
spacewander merged pull request #5589:
URL: https://github.com/apache/apisix/pull/5589
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] shuaijinchao commented on a change in pull request #5589: fix: invalid error after passive health check is changed
Posted by GitBox <gi...@apache.org>.
shuaijinchao commented on a change in pull request #5589:
URL: https://github.com/apache/apisix/pull/5589#discussion_r755634250
##########
File path: apisix/schema_def.lua
##########
@@ -259,24 +259,38 @@ local health_checker = {
},
tcp_failures = {
type = "integer",
- minimum = 1,
+ minimum = 0,
maximum = 254,
default = 2
},
timeouts = {
type = "integer",
- minimum = 1,
+ minimum = 0,
maximum = 254,
default = 7
},
http_failures = {
type = "integer",
- minimum = 1,
+ minimum = 0,
maximum = 254,
default = 5
},
}
}
+ },
+ default = {
+ type = "http",
+ healthy = {
+ http_statuses = { 200, 201, 202, 203, 204, 205, 206, 207, 208, 226,
+ 300, 301, 302, 303, 304, 305, 306, 307, 308 },
+ successes = 0,
+ },
+ unhealthy = {
+ http_statuses = { 429, 500, 503 },
+ tcp_failures = 0,
Review comment:
document updated
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] shuaijinchao commented on a change in pull request #5589: fix: invalid error after passive health check is changed
Posted by GitBox <gi...@apache.org>.
shuaijinchao commented on a change in pull request #5589:
URL: https://github.com/apache/apisix/pull/5589#discussion_r755626947
##########
File path: apisix/schema_def.lua
##########
@@ -259,24 +259,38 @@ local health_checker = {
},
tcp_failures = {
type = "integer",
- minimum = 1,
+ minimum = 0,
maximum = 254,
default = 2
},
timeouts = {
type = "integer",
- minimum = 1,
+ minimum = 0,
maximum = 254,
default = 7
},
http_failures = {
type = "integer",
- minimum = 1,
+ minimum = 0,
maximum = 254,
default = 5
},
}
}
+ },
+ default = {
+ type = "http",
+ healthy = {
+ http_statuses = { 200, 201, 202, 203, 204, 205, 206, 207, 208, 226,
+ 300, 301, 302, 303, 304, 305, 306, 307, 308 },
+ successes = 0,
+ },
+ unhealthy = {
+ http_statuses = { 429, 500, 503 },
+ tcp_failures = 0,
Review comment:
Changing the value to 0 will disable the passive health check, which is necessary when only the active check is configured, documentation needs to be updated
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] shuaijinchao commented on a change in pull request #5589: fix: invalid error after passive health check is changed
Posted by GitBox <gi...@apache.org>.
shuaijinchao commented on a change in pull request #5589:
URL: https://github.com/apache/apisix/pull/5589#discussion_r755631478
##########
File path: t/admin/health-check.t
##########
@@ -469,17 +434,64 @@ GET /t
ngx.print(body)
}
}
---- request
-GET /t
--- error_code: 400
--- response_body
{"error_msg":"invalid configuration: property \"upstream\" validation failed: property \"checks\" validation failed: object matches none of the requireds: [\"active\"] or [\"active\",\"passive\"]"}
---- no_error_log
-[error]
-=== TEST 13: number type timeout
+=== TEST 13: only active
Review comment:
The purpose of this test is to verify that the default passive configuration will be added in the case of active only. I think this title description is not very accurate, what do you think?
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org