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/05/31 11:07:25 UTC

[GitHub] [apisix] tokers commented on a change in pull request #4340: feat: allow to set custom timeout for route

tokers commented on a change in pull request #4340:
URL: https://github.com/apache/apisix/pull/4340#discussion_r642156329



##########
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:
       ```suggestion
   | upstream_timeout | False                                    | Upstream    | Set the upstream timeout for connecting, 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}              |
   ```

##########
File path: apisix/schema_def.lua
##########
@@ -491,6 +491,15 @@ _M.route = {
             minItems = 1,
             uniqueItems = true,
         },
+        upstream_timeout = {

Review comment:
       May abstract the timeout schema so it can be reused by route and upstream schema.

##########
File path: t/admin/routes2.t
##########
@@ -650,3 +650,40 @@ GET /t
 {"error_msg":"failed to fetch plugin config info by plugin config id [not_found], response code: 404"}
 --- no_error_log
 [error]
+
+
+
+=== TEST 18: valid route with upstream_timeout

Review comment:
       Need further test cases to cover the usage of `upstream_timeout`.




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