srijeet0406 commented on a change in pull request #5512:
URL: https://github.com/apache/trafficcontrol/pull/5512#discussion_r604510550
##########
File path: traffic_ops/testing/api/v4/roles_test.go
##########
@@ -165,7 +166,7 @@ func UpdateTestRoles(t *testing.T) {
t.Logf("testData.Roles contains: %+v\n", testData.Roles)
firstRole := testData.Roles[0]
Review comment:
Could you add a `len` check for `testData.Roles` and maybe a `nil` check
for the `Name` and `ID` later?
##########
File path: traffic_ops/v4-client/cdn.go
##########
@@ -35,73 +36,49 @@ func (to *Session) CreateCDN(cdn tc.CDN) (tc.Alerts,
toclientlib.ReqInf, error)
return alerts, reqInf, err
}
-func (to *Session) UpdateCDNByIDWithHdr(id int, cdn tc.CDN, header
http.Header) (tc.Alerts, toclientlib.ReqInf, error) {
+// UpdateCDNByID replaces the identified CDN with the provided CDN.
+func (to *Session) UpdateCDNByID(id int, cdn tc.CDN, header http.Header)
(tc.Alerts, toclientlib.ReqInf, error) {
route := fmt.Sprintf("%s/%d", APICDNs, id)
var alerts tc.Alerts
reqInf, err := to.put(route, cdn, header, &alerts)
return alerts, reqInf, err
}
-// UpdateCDNByID updates a CDN by ID.
-// Deprecated: UpdateCDNByID will be removed in 6.0. Use UpdateCDNByIDWithHdr.
-func (to *Session) UpdateCDNByID(id int, cdn tc.CDN) (tc.Alerts,
toclientlib.ReqInf, error) {
- return to.UpdateCDNByIDWithHdr(id, cdn, nil)
-}
-
-func (to *Session) GetCDNsWithHdr(header http.Header) ([]tc.CDN,
toclientlib.ReqInf, error) {
+// GetCDNs retrieves all CDNs in Traffic Ops.
+func (to *Session) GetCDNs(header http.Header) ([]tc.CDN, toclientlib.ReqInf,
error) {
var data tc.CDNsResponse
reqInf, err := to.get(APICDNs, header, &data)
return data.Response, reqInf, err
}
-// GetCDNs eturns a list of CDNs.
-// Deprecated: GetCDNs will be removed in 6.0. Use GetCDNsWithHdr.
-func (to *Session) GetCDNs() ([]tc.CDN, toclientlib.ReqInf, error) {
- return to.GetCDNsWithHdr(nil)
-}
-
-func (to *Session) GetCDNByIDWithHdr(id int, header http.Header) ([]tc.CDN,
toclientlib.ReqInf, error) {
+// GetCDNByID retrieves the CDN with the given ID.
+func (to *Session) GetCDNByID(id int, header http.Header) ([]tc.CDN,
toclientlib.ReqInf, error) {
route := fmt.Sprintf("%s?id=%v", APICDNs, id)
var data tc.CDNsResponse
reqInf, err := to.get(route, header, &data)
return data.Response, reqInf, err
}
-// GetCDNByID a CDN by the CDN ID.
-// Deprecated: GetCDNByID will be removed in 6.0. Use GetCDNByIDWithHdr.
-func (to *Session) GetCDNByID(id int) ([]tc.CDN, toclientlib.ReqInf, error) {
- return to.GetCDNByIDWithHdr(id, nil)
-}
-
-func (to *Session) GetCDNByNameWithHdr(name string, header http.Header)
([]tc.CDN, toclientlib.ReqInf, error) {
+// GetCDNByName retrieves the CDN with the given Name.
+func (to *Session) GetCDNByName(name string, header http.Header) ([]tc.CDN,
toclientlib.ReqInf, error) {
route := fmt.Sprintf("%s?name=%s", APICDNs, url.QueryEscape(name))
var data tc.CDNsResponse
reqInf, err := to.get(route, header, &data)
return data.Response, reqInf, err
}
-// GetCDNByName gets a CDN by the CDN name.
-// Deprecated: GetCDNByName will be removed in 6.0. Use GetCDNByNameWithHdr.
-func (to *Session) GetCDNByName(name string) ([]tc.CDN, toclientlib.ReqInf,
error) {
- return to.GetCDNByNameWithHdr(name, nil)
-}
-
-// DeleteCDNByID deletes a CDN by ID.
+// DeleteCDNByID deletes the CDN with the given ID.
func (to *Session) DeleteCDNByID(id int) (tc.Alerts, toclientlib.ReqInf,
error) {
Review comment:
This could just be `DeleteCDN`
##########
File path: traffic_ops/testing/api/v4/federation_resolvers_test.go
##########
@@ -161,7 +170,7 @@ func CreateTestFederationResolvers(t *testing.T) {
fr.TypeID = util.UIntPtr(uint(tid[0].ID))
- alerts, _, err := TOSession.CreateFederationResolver(fr)
+ alerts, _, err := TOSession.CreateFederationResolver(fr, nil)
Review comment:
I dont think any of the other `Create` client methods are passing in the
header. That said, I think that would be helpful and we should make the other
`Create` methods also take in the `header` parameter.
##########
File path: traffic_ops/v4-client/coordinate.go
##########
@@ -24,68 +24,50 @@ import (
)
const (
+ // APICoordinates is the API version-relative path for the /coordinates
API endpoint.
APICoordinates = "/coordinates"
)
-// Create a Coordinate
+// CreateCoordinate creates the given Coordinate.
func (to *Session) CreateCoordinate(coordinate tc.Coordinate) (tc.Alerts,
toclientlib.ReqInf, error) {
var alerts tc.Alerts
reqInf, err := to.post(APICoordinates, coordinate, nil, &alerts)
return alerts, reqInf, err
}
-func (to *Session) UpdateCoordinateByIDWithHdr(id int, coordinate
tc.Coordinate, header http.Header) (tc.Alerts, toclientlib.ReqInf, error) {
+// UpdateCoordinate replaces the Coordinate with the given ID with the one
+// provided.
+func (to *Session) UpdateCoordinate(id int, coordinate tc.Coordinate, header
http.Header) (tc.Alerts, toclientlib.ReqInf, error) {
route := fmt.Sprintf("%s?id=%d", APICoordinates, id)
var alerts tc.Alerts
reqInf, err := to.put(route, coordinate, header, &alerts)
return alerts, reqInf, err
}
-// Update a Coordinate by ID
-// Deprecated: UpdateCoordinateByID will be removed in 6.0. Use
UpdateCoordinateByIDWithHdr.
-func (to *Session) UpdateCoordinateByID(id int, coordinate tc.Coordinate)
(tc.Alerts, toclientlib.ReqInf, error) {
- return to.UpdateCoordinateByIDWithHdr(id, coordinate, nil)
-}
-
-func (to *Session) GetCoordinatesWithHdr(header http.Header) ([]tc.Coordinate,
toclientlib.ReqInf, error) {
+// GetCoordinates returns all Coordinates in Traffic Ops.
+func (to *Session) GetCoordinates(header http.Header) ([]tc.Coordinate,
toclientlib.ReqInf, error) {
var data tc.CoordinatesResponse
reqInf, err := to.get(APICoordinates, header, &data)
return data.Response, reqInf, err
}
-// Returns a list of Coordinates
-// Deprecated: GetCoordinates will be removed in 6.0. Use
GetCoordinatesWithHdr.
-func (to *Session) GetCoordinates() ([]tc.Coordinate, toclientlib.ReqInf,
error) {
- return to.GetCoordinatesWithHdr(nil)
-}
-
-func (to *Session) GetCoordinateByIDWithHdr(id int, header http.Header)
([]tc.Coordinate, toclientlib.ReqInf, error) {
+// GetCoordinateByID retrieves the Coordinate with the given ID.
+func (to *Session) GetCoordinateByID(id int, header http.Header)
([]tc.Coordinate, toclientlib.ReqInf, error) {
route := fmt.Sprintf("%s?id=%d", APICoordinates, id)
var data tc.CoordinatesResponse
reqInf, err := to.get(route, header, &data)
return data.Response, reqInf, err
}
-// GET a Coordinate by the Coordinate id
-// Deprecated: GetCoordinateByID will be removed in 6.0. Use
GetCoordinateByIDWithHdr.
-func (to *Session) GetCoordinateByID(id int) ([]tc.Coordinate,
toclientlib.ReqInf, error) {
- return to.GetCoordinateByIDWithHdr(id, nil)
-}
-
-// GET a Coordinate by the Coordinate name
-// Deprecated: GetCoordinateByName will be removed in 6.0. Use
GetCoordinateByNameWithHdr.
-func (to *Session) GetCoordinateByName(name string) ([]tc.Coordinate,
toclientlib.ReqInf, error) {
- return to.GetCoordinateByNameWithHdr(name, nil)
-}
-
-func (to *Session) GetCoordinateByNameWithHdr(name string, header http.Header)
([]tc.Coordinate, toclientlib.ReqInf, error) {
+// GetCoordinateByName retrieves the Coordinate with the given Name.
+func (to *Session) GetCoordinateByName(name string, header http.Header)
([]tc.Coordinate, toclientlib.ReqInf, error) {
route := fmt.Sprintf("%s?name=%s", APICoordinates, name)
var data tc.CoordinatesResponse
reqInf, err := to.get(route, header, &data)
return data.Response, reqInf, err
}
-// DELETE a Coordinate by ID
+// DeleteCoordinateByID deletes the Coordinate with the given ID.
Review comment:
Same comment here
##########
File path: traffic_ops/testing/api/v4/cachegroups_test.go
##########
@@ -112,7 +112,7 @@ func GetTestCacheGroupsByShortNameIMS(t *testing.T) {
time := futureTime.Format(time.RFC1123)
header.Set(rfc.IfModifiedSince, time)
for _, cg := range testData.CacheGroups {
- _, reqInf, err :=
TOSession.GetCacheGroupNullableByShortNameWithHdr(*cg.ShortName, header)
+ _, reqInf, err :=
TOSession.GetCacheGroupByShortName(*cg.ShortName, header)
Review comment:
could we add a nil check for cg.ShortName here?
##########
File path: traffic_ops/v4-client/cdnfederations.go
##########
@@ -29,58 +29,44 @@ import (
* `/cdns/:name/federations`. Although the behavior is odd, it is kept to
* keep the same behavior from perl. */
+// CreateCDNFederationByName creates the given Federation in the CDN with the
+// given name.
func (to *Session) CreateCDNFederationByName(f tc.CDNFederation, CDNName
string) (*tc.CreateCDNFederationResponse, toclientlib.ReqInf, error) {
data := tc.CreateCDNFederationResponse{}
route := "/cdns/" + url.QueryEscape(CDNName) + "/federations"
inf, err := to.post(route, f, nil, &data)
return &data, inf, err
}
-func (to *Session) GetCDNFederationsByNameWithHdr(CDNName string, header
http.Header) (*tc.CDNFederationResponse, toclientlib.ReqInf, error) {
+// GetCDNFederationsByName retrieves all Federations in the CDN with the given
+// name.
+func (to *Session) GetCDNFederationsByName(CDNName string, header http.Header)
(*tc.CDNFederationResponse, toclientlib.ReqInf, error) {
data := tc.CDNFederationResponse{}
route := "/cdns/" + url.QueryEscape(CDNName) + "/federations"
inf, err := to.get(route, header, &data)
return &data, inf, err
}
-// Deprecated: GetCDNFederationsByName will be removed in 6.0. Use
GetCDNFederationsByNameWithHdr.
-func (to *Session) GetCDNFederationsByName(CDNName string)
(*tc.CDNFederationResponse, toclientlib.ReqInf, error) {
- return to.GetCDNFederationsByNameWithHdr(CDNName, nil)
-}
-
-func (to *Session) GetCDNFederationsByNameWithHdrReturnList(CDNName string,
header http.Header) ([]tc.CDNFederation, toclientlib.ReqInf, error) {
- route := "/cdns/" + url.QueryEscape(CDNName) + "/federations"
- resp := struct {
- Response []tc.CDNFederation `json:"response"`
- }{}
- inf, err := to.get(route, header, &resp)
- return resp.Response, inf, err
-}
-
-func (to *Session) GetCDNFederationsByIDWithHdr(CDNName string, ID int, header
http.Header) (*tc.CDNFederationResponse, toclientlib.ReqInf, error) {
+// GetCDNFederationsByID retrieves the Federation in the CDN with the given
+// name that has the given ID.
+func (to *Session) GetCDNFederationsByID(CDNName string, ID int, header
http.Header) (*tc.CDNFederationResponse, toclientlib.ReqInf, error) {
data := tc.CDNFederationResponse{}
route := fmt.Sprintf("/cdns/%s/federations?id=%v",
url.QueryEscape(CDNName), ID)
inf, err := to.get(route, header, &data)
return &data, inf, err
}
-// Deprecated: GetCDNFederationsByID will be removed in 6.0. Use
GetCDNFederationsByIDWithHdr.
-func (to *Session) GetCDNFederationsByID(CDNName string, ID int)
(*tc.CDNFederationResponse, toclientlib.ReqInf, error) {
- return to.GetCDNFederationsByIDWithHdr(CDNName, ID, nil)
-}
-
-func (to *Session) UpdateCDNFederationsByIDWithHdr(f tc.CDNFederation, CDNName
string, ID int, h http.Header) (*tc.UpdateCDNFederationResponse,
toclientlib.ReqInf, error) {
+// UpdateCDNFederation replaces the Federation with the given ID in the CDN
+// with the given name with the provided Federation.
+func (to *Session) UpdateCDNFederation(f tc.CDNFederation, CDNName string, ID
int, h http.Header) (*tc.UpdateCDNFederationResponse, toclientlib.ReqInf,
error) {
data := tc.UpdateCDNFederationResponse{}
route := fmt.Sprintf("/cdns/%s/federations/%d",
url.QueryEscape(CDNName), ID)
inf, err := to.put(route, f, h, &data)
return &data, inf, err
}
-// Deprecated: UpdateCDNFederationsByID will be removed in 6.0. Use
UpdateCDNFederationsByIDWithHdr.
-func (to *Session) UpdateCDNFederationsByID(f tc.CDNFederation, CDNName
string, ID int) (*tc.UpdateCDNFederationResponse, toclientlib.ReqInf, error) {
- return to.UpdateCDNFederationsByIDWithHdr(f, CDNName, ID, nil)
-}
-
+// DeleteCDNFederationByID deletes the Federation with the given ID in the CDN
+// with the given name.
func (to *Session) DeleteCDNFederationByID(CDNName string, ID int)
(*tc.DeleteCDNFederationResponse, toclientlib.ReqInf, error) {
Review comment:
Should this just be `DeleteCDNFederation`?
##########
File path: traffic_ops/testing/api/v4/origins_test.go
##########
@@ -101,7 +102,7 @@ func NotFoundDeleteTest(t *testing.T) {
}
func GetTestOrigins(t *testing.T) {
- _, _, err := TOSession.GetOrigins()
+ _, _, err := TOSession.GetOrigins(nil)
Review comment:
This might not be your code, but any reason not to pass in the `header`
argument in the `GetOrigins` call?
##########
File path: CHANGELOG.md
##########
@@ -34,7 +34,6 @@ The format is based on [Keep a
Changelog](http://keepachangelog.com/en/1.0.0/).
- [#5609](https://github.com/apache/trafficcontrol/issues/5609) - Fixed GET
/servercheck filter for an extra query param.
- [#5565](https://github.com/apache/trafficcontrol/issues/5565) - TO GET
/caches/stats panic converting string to uint64
- [#5558](https://github.com/apache/trafficcontrol/issues/5558) - Fixed `TM
UI` and `/api/cache-statuses` to report aggregate `bandwidth_kbps` correctly.
-- [#5288](https://github.com/apache/trafficcontrol/issues/5288) - Fixed the
ability to create and update a server with MTU value >= 1280.
Review comment:
Should this be a part of this PR?
--
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]
