tokers commented on a change in pull request #3512:
URL: https://github.com/apache/apisix/pull/3512#discussion_r571512362
##########
File path: apisix/init.lua
##########
@@ -409,6 +409,33 @@ function _M.http_access_phase()
api_ctx.route_id = route.value.id
api_ctx.route_name = route.value.name
+ if route.value.script then
Review comment:
Why run this if and else blocks eariler?
##########
File path: apisix/plugins/traffic-split.lua
##########
@@ -258,18 +258,21 @@ end
local function new_rr_obj(weighted_upstreams)
local server_list = {}
for i, upstream_obj in ipairs(weighted_upstreams) do
- if not upstream_obj.upstream then
+ if not upstream_obj.upstream and not upstream_obj.upstream_id then
-- If the `upstream` object has only the `weight` value, it means
-- that the `upstream` weight value on the default `route` has
been reached.
- -- Need to set an identifier to mark the empty upstream.
- upstream_obj.upstream = "empty_upstream"
+ -- Mark empty upstream services in the plugin, but with weight.
+ upstream_obj.upstream = "plugin#upstream#is#empty"
+ server_list[upstream_obj.upstream] = upstream_obj.weight
end
- if type(upstream_obj.upstream) == "table" then
+ if upstream_obj.upstream_id then
Review comment:
Where is the definition of the `upstream_id`?
##########
File path: doc/plugins/traffic-split.md
##########
@@ -111,6 +113,40 @@ curl http://127.0.0.1:9080/apisix/admin/routes/1 -H
'X-API-KEY: edd1c9f034335f13
}'
```
+2. Binding upstream services through the `upstream_id` attribute in the plugin.
Review comment:
Don't use the term "services" here, people might be confused it with the
APISIX Service.
##########
File path: apisix/plugins/traffic-split.lua
##########
@@ -258,18 +258,21 @@ end
local function new_rr_obj(weighted_upstreams)
local server_list = {}
for i, upstream_obj in ipairs(weighted_upstreams) do
- if not upstream_obj.upstream then
+ if not upstream_obj.upstream and not upstream_obj.upstream_id then
-- If the `upstream` object has only the `weight` value, it means
-- that the `upstream` weight value on the default `route` has
been reached.
- -- Need to set an identifier to mark the empty upstream.
- upstream_obj.upstream = "empty_upstream"
+ -- Mark empty upstream services in the plugin, but with weight.
+ upstream_obj.upstream = "plugin#upstream#is#empty"
+ server_list[upstream_obj.upstream] = upstream_obj.weight
end
- if type(upstream_obj.upstream) == "table" then
+ if upstream_obj.upstream_id then
+ server_list[upstream_obj.upstream_id] = upstream_obj.weight
+ elseif type(upstream_obj.upstream) == "table" then
Review comment:
Seems the schema of upstream will guard the type of upstream and we
don't check the type again here.
##########
File path: apisix/plugins/traffic-split.lua
##########
@@ -258,18 +258,21 @@ end
local function new_rr_obj(weighted_upstreams)
local server_list = {}
for i, upstream_obj in ipairs(weighted_upstreams) do
- if not upstream_obj.upstream then
+ if not upstream_obj.upstream and not upstream_obj.upstream_id then
-- If the `upstream` object has only the `weight` value, it means
-- that the `upstream` weight value on the default `route` has
been reached.
- -- Need to set an identifier to mark the empty upstream.
- upstream_obj.upstream = "empty_upstream"
+ -- Mark empty upstream services in the plugin, but with weight.
+ upstream_obj.upstream = "plugin#upstream#is#empty"
+ server_list[upstream_obj.upstream] = upstream_obj.weight
Review comment:
Could we unify the key type of `server_list`, we have two types of the
key, i.e. string, table, and we have a special empty upstream mark.
----------------------------------------------------------------
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:
us...@infra.apache.org
