Yiyiyimu commented on a change in pull request #4340:
URL: https://github.com/apache/apisix/pull/4340#discussion_r642135083
##########
File path: apisix/schema_def.lua
##########
@@ -491,6 +491,15 @@ _M.route = {
minItems = 1,
uniqueItems = true,
},
+ upstream_timeout = {
Review comment:
Just use `timeout` would seem a better name. It would override upstream
timeout but it's still for routes but not upstreams
##########
File path: apisix/balancer.lua
##########
@@ -136,8 +136,18 @@ end
-- set_balancer_opts will be called in balancer phase and before any tries
local function set_balancer_opts(ctx)
local up_conf = ctx.upstream_conf
- if up_conf.timeout then
- local timeout = up_conf.timeout
+
+ -- If the matched route has timeout config, prefer to use the route config.
+ local route_conf = ctx.matched_route.value
Review comment:
maybe need to check if `ctx.matched_route` is nil here
##########
File path: docs/en/latest/admin-api.md
##########
@@ -88,6 +88,7 @@ Note: When the `Admin API` is enabled, it will occupy the API
prefixed with `/ap
| script | False | Script |
See [Script](architecture-design/script.md) for more
| |
| upstream | False | Upstream |
Enabled Upstream configuration, see [Upstream](architecture-design/upstream.md)
for more
| |
| upstream_id | False | Upstream |
Enabled upstream id, see [Upstream](architecture-design/upstream.md) for more
| |
+| upstream_timeout | False | Upstream |
Set the upstream timeout for connection, sending and receiving messages of the
route. This option will overwrite the [timeout](#upstream) option which set in
upstream configuration.
| {"connect": 3, "send": 3, "read": 3} |
Review comment:
ditto and some other places. Also the type maybe should be `Auxiliary`?
I'm not sure about this
--
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:
[email protected]
