zrhoffman commented on a change in pull request #5538:
URL: https://github.com/apache/trafficcontrol/pull/5538#discussion_r579413317
##########
File path: traffic_ops/traffic_ops_golang/server/servers.go
##########
@@ -1119,10 +1124,15 @@ func getServers(h http.Header, params
map[string]string, tx *sqlx.Tx, user *auth
// if ds requested uses mid-tier caches, add those to the list as well
if usesMids {
- midIDs, userErr, sysErr, errCode := getMidServers(ids, servers,
dsID, cdnID, tx)
+ includeCapabilities := false
+ if _, ok := params["dsId"]; ok && dsHasRequiredCapabilities {
+ includeCapabilities = true
+ }
+ midIDs, userErr, sysErr, errCode := getMidServers(ids, servers,
dsID, cdnID, tx, includeCapabilities)
Review comment:
`dsHasRequiredCapabilities` can only be set if `params["dsId"]` exists,
so `includeCapabilities` is always equal to `dsHasRequiredCapabilities`.
##########
File path: traffic_ops/traffic_ops_golang/server/servers.go
##########
@@ -1237,6 +1248,22 @@ func getMidServers(edgeIDs []int, servers
map[int]tc.ServerV40, dsID int, cdnID
FROM deliveryservice d
WHERE d.id = :ds_id) IS NULL
`
+ query = selectQuery + `
+ WHERE (SELECT ARRAY_AGG(ssc.server_capability) FROM
server_server_capability ssc WHERE ssc."server" = ANY(
+ ` + q + `)) @> (SELECT ARRAY_AGG(drc.required_capability) FROM
deliveryservices_required_capability drc WHERE drc.deliveryservice_id = :ds_id)`
Review comment:
Can this query be formatted nicer so we can tell what's going on here?
Commenting the query would not hurt, either.
##########
File path: traffic_ops/traffic_ops_golang/server/servers.go
##########
@@ -1237,6 +1248,22 @@ func getMidServers(edgeIDs []int, servers
map[int]tc.ServerV40, dsID int, cdnID
FROM deliveryservice d
WHERE d.id = :ds_id) IS NULL
`
+ query = selectQuery + `
+ WHERE (SELECT ARRAY_AGG(ssc.server_capability) FROM
server_server_capability ssc WHERE ssc."server" = ANY(
+ ` + q + `)) @> (SELECT ARRAY_AGG(drc.required_capability) FROM
deliveryservices_required_capability drc WHERE drc.deliveryservice_id = :ds_id)`
+ } else {
+ // TODO: include secondary parent?
+ query = selectQuery + `
+ WHERE t.name = :cache_type_mid AND s.cachegroup IN (
+ SELECT cg.parent_cachegroup_id FROM cachegroup AS cg
+ WHERE cg.id IN (
+ SELECT s.cachegroup FROM server AS s
+ WHERE s.id = ANY(:edge_ids)))
+ AND (SELECT d.topology
+ FROM deliveryservice d
+ WHERE d.id = :ds_id) IS NULL
+ `
Review comment:
The `WHERE` clause in `query` is identical to `WHERE` clause in `q` in
https://github.com/apache/trafficcontrol/blob/84dca54e98665ed36d9a1607fe4cb9997a262913/traffic_ops/traffic_ops_golang/server/servers.go#L1241-L1250
Can the `WHERE` clause be moved outside the `if` statement so it only needs
to be declared once?
----------------------------------------------------------------
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]
