rob05c commented on a change in pull request #3104: Add monitoring.json
snapshotting
URL: https://github.com/apache/trafficcontrol/pull/3104#discussion_r240321578
##########
File path: traffic_ops/traffic_ops_golang/routes.go
##########
@@ -142,7 +141,8 @@ func Routes(d ServerData) ([]Route, []RawRoute,
http.Handler, error) {
{1.4, http.MethodGet, `cdns/dnsseckeys/refresh/?(\.json)?$`,
cdn.RefreshDNSSECKeys, auth.PrivLevelOperations, Authenticated, nil},
//CDN: Monitoring: Traffic Monitor
- {1.1, http.MethodGet,
`cdns/{cdn}/configs/monitoring(\.json)?$`, monitoring.Get,
auth.PrivLevelReadOnly, Authenticated, nil},
+ {1.1, http.MethodGet,
`cdns/{cdn}/configs/monitoring(\.json)?$`,
crconfig.SnapshotGetMonitoringHandler, auth.PrivLevelReadOnly, Authenticated,
nil},
+ {1.2, http.MethodGet,
`cdns/{name}/configs/monitoring(\.json)?$`,
crconfig.SnapshotGetMonitoringHandler, auth.PrivLevelReadOnly, Authenticated,
nil},
Review comment:
Actually, the second one is both broken, and will never get called. Routes
are called in the order they're registered. So, a 1.2 route has to be before a
1.1 route with the same path, or it'll never get called. But it's broken
anyway, because it's using `{name}`, but the handler itself expected a param
named `{cdn}`. So yeah, needs removed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
