zrhoffman commented on code in PR #7073:
URL: https://github.com/apache/trafficcontrol/pull/7073#discussion_r974574516
##########
traffic_ops/traffic_ops_golang/deliveryservice/eligible.go:
##########
@@ -147,7 +156,8 @@ t.name as server_type,
s.type as server_type_id,
s.config_update_time > s.config_apply_time AS upd_pending,
ARRAY(select ssc.server_capability from server_server_capability ssc where
ssc.server = s.id order by ssc.server_capability) as server_capabilities,
-ARRAY(select drc.required_capability from deliveryservices_required_capability
drc where drc.deliveryservice_id = (select v from ds_id) order by
drc.required_capability) as deliveryservice_capabilities
+ARRAY(select drc.required_capability from deliveryservices_required_capability
drc where drc.deliveryservice_id = (select v from ds_id) order by
drc.required_capability) as deliveryservice_capabilities,
+(SELECT ARRAY_AGG(asn) AS asns FROM asn a WHERE a.cachegroup = s.cachegroup)
AS asns
Review Comment:
This should be a join on `asns`, joins have better performance than
subqueries
##########
traffic_ops/traffic_ops_golang/deliveryservice/servers/servers.go:
##########
@@ -758,7 +766,8 @@ s.status as status_id,
s.tcp_port,
t.name as server_type,
s.type as server_type_id,
-s.config_update_time > s.config_apply_time AS upd_pending
+s.config_update_time > s.config_apply_time AS upd_pending,
+(SELECT ARRAY_AGG(asn) AS asns FROM asn a WHERE a.cachegroup = s.cachegroup)
AS asns
Review Comment:
This should be a join on `asns`, as joins have better performance than
subqueries
##########
traffic_ops/traffic_ops_golang/deliveryservice/servers/servers_test.go:
##########
@@ -192,10 +194,13 @@ func getMockDSServers() []tc.DSServerV4 {
CDNName: util.StrPtr("cdnTest"),
DomainName: util.StrPtr("domain"),
}
- srv := tc.DSServerV4{
+ srvV40 := tc.DSServerV40{
DSServerBaseV4: base,
ServerInterfaces: &[]tc.ServerInterfaceInfoV40{}, // left empty
because it must be written as json above since sqlmock does not support nested
arrays
}
+ srv := tc.DSServerV4{
+ DSServerV40: srvV40,
+ }
Review Comment:
This part gets messy each time the struct version changes and has come up
before (see
https://github.com/apache/trafficcontrol/pull/4692#discussion_r426898413). If
each field is assigned separately like
```go
srv := tc.DSServerV4{}
srv.ID = util.IntPtr(1)
srv.Cachegroup = util.StrPtr("cgTest")
// (and so on for the other fields)
srv.ServerInterfaces = &[]tc.ServerInterfaceInfoV40{}
```
then that can be avoided.
##########
docs/source/api/v5/deliveryservices_id_servers.rst:
##########
@@ -142,6 +145,7 @@ Response Structure
"type": "EDGE",
"typeId": 11,
"updPending": false,
+ "asns":[1,2],
Review Comment:
Nit: Needs jq formatting
##########
traffic_ops/traffic_ops_golang/deliveryservice/eligible_test.go:
##########
@@ -84,7 +84,8 @@ func TestGetEligibleServers(t *testing.T) {
"server_type_id",
"upd_pending",
"server_capabilities",
- "deliveryservice_capabilities"}
+ "deliveryservice_capabilities",
+ "asns"}
Review Comment:
Nit: if the `}` goes on its own line, no existing lines will need to be
changed the next time a field is added
##########
traffic_ops/traffic_ops_golang/deliveryservice/servers/servers_test.go:
##########
@@ -115,7 +115,8 @@ func TestReadServers(t *testing.T) {
"tcp_port",
"server_type",
"server_type_id",
- "upd_pending"}
+ "upd_pending",
+ "asns"}
Review Comment:
Nit: if the `}` goes on its own line, no existing lines will need to be
changed the next time a field is added
##########
docs/source/api/v4/deliveryservices_id_servers.rst:
##########
@@ -142,6 +145,7 @@ Response Structure
"type": "EDGE",
"typeId": 11,
"updPending": false,
+ "asns":[1,2],
Review Comment:
Nit: Needs jq formatting
##########
docs/source/api/v4/deliveryservices_id_servers_eligible.rst:
##########
@@ -140,6 +143,7 @@ Response Structure
"type": "EDGE",
"typeId": 11,
"updPending": false,
+ "asns":[1,2],
Review Comment:
Nit: Needs jq formatting
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
