You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@trafficcontrol.apache.org by GitBox <gi...@apache.org> on 2022/07/08 15:30:31 UTC

[GitHub] [trafficcontrol] rawlinp commented on a diff in pull request #6945: Remove Traffic Ops API version 2

rawlinp commented on code in PR #6945:
URL: https://github.com/apache/trafficcontrol/pull/6945#discussion_r916923991


##########
traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go:
##########
@@ -608,43 +580,15 @@ func (ds *TODeliveryService) Read(h http.Header, useIMS bool) ([]interface{}, er
 		// NOTE: it's required to handle minor version cases in a descending >= manner
 		case version.Major > 3:
 			returnable = append(returnable, ds.RemoveLD1AndLD2())
-		case version.Major > 2 && version.Minor >= 1:
+		case version.Major == 3 && version.Minor >= 1:
 			returnable = append(returnable, ds.DowngradeToV31())
-		case version.Major > 2:
-			returnable = append(returnable, ds.DowngradeToV31().DeliveryServiceV30)

Review Comment:
   I think this part may be important for differentiating v3.0 and v3.1



##########
traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go:
##########
@@ -128,34 +128,6 @@ func GetDSTLSVersions(dsID int, tx *sql.Tx) ([]string, error) {
 	return vers, err
 }
 
-// CreateV15 is the POST handler for APIv2's deliveryservices endpoint, named
-// with "V15" for legacy reasons.
-// TODO allow users to post names (type, cdn, etc) and get the IDs from the
-// names. This isn't trivial to do in a single query, without dynamically
-// building the entire insert query, and ideally inserting would be one query.
-// But it'd be much more convenient for users. Alternatively, remove IDs from
-// the database entirely and use real candidate keys.
-func CreateV15(w http.ResponseWriter, r *http.Request) {
-	inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil)
-	if userErr != nil || sysErr != nil {
-		api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
-		return
-	}
-	defer inf.Close()
-
-	ds := tc.DeliveryServiceNullableV15{}
-	if err := json.NewDecoder(r.Body).Decode(&ds); err != nil {
-		api.HandleErr(w, r, inf.Tx.Tx, http.StatusBadRequest, errors.New("decoding: "+err.Error()), nil)
-		return
-	}
-
-	res, status, userErr, sysErr := createV15(w, r, inf, ds)

Review Comment:
   I believe you can also remove the `createV15` function now too



##########
traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go:
##########
@@ -608,43 +580,15 @@ func (ds *TODeliveryService) Read(h http.Header, useIMS bool) ([]interface{}, er
 		// NOTE: it's required to handle minor version cases in a descending >= manner
 		case version.Major > 3:
 			returnable = append(returnable, ds.RemoveLD1AndLD2())
-		case version.Major > 2 && version.Minor >= 1:
+		case version.Major == 3 && version.Minor >= 1:
 			returnable = append(returnable, ds.DowngradeToV31())
-		case version.Major > 2:
-			returnable = append(returnable, ds.DowngradeToV31().DeliveryServiceV30)
-		case version.Major > 1:
-			returnable = append(returnable, ds.DowngradeToV31().DeliveryServiceNullableV15)
 		default:
 			return nil, nil, fmt.Errorf("TODeliveryService.Read called with invalid API version: %d.%d", version.Major, version.Minor), http.StatusInternalServerError, nil
 		}
 	}
 	return returnable, nil, nil, errCode, maxTime
 }
 
-func UpdateV15(w http.ResponseWriter, r *http.Request) {
-	inf, userErr, sysErr, errCode := api.NewInfo(r, nil, []string{"id"})
-	if userErr != nil || sysErr != nil {
-		api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
-		return
-	}
-	defer inf.Close()
-
-	id := inf.IntParams["id"]
-
-	ds := tc.DeliveryServiceNullableV15{}
-	if err := json.NewDecoder(r.Body).Decode(&ds); err != nil {
-		api.HandleErr(w, r, inf.Tx.Tx, http.StatusBadRequest, errors.New("malformed JSON: "+err.Error()), nil)
-		return
-	}
-	ds.ID = &id
-
-	res, status, userErr, sysErr := updateV15(w, r, inf, &ds)

Review Comment:
   I believe you can also remove the `updateV15` function as well



##########
traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go:
##########
@@ -128,34 +128,6 @@ func GetDSTLSVersions(dsID int, tx *sql.Tx) ([]string, error) {
 	return vers, err
 }
 
-// CreateV15 is the POST handler for APIv2's deliveryservices endpoint, named
-// with "V15" for legacy reasons.
-// TODO allow users to post names (type, cdn, etc) and get the IDs from the
-// names. This isn't trivial to do in a single query, without dynamically
-// building the entire insert query, and ideally inserting would be one query.
-// But it'd be much more convenient for users. Alternatively, remove IDs from
-// the database entirely and use real candidate keys.
-func CreateV15(w http.ResponseWriter, r *http.Request) {

Review Comment:
   I see Create and Update being removed, but I believe there is also a switch case in the `Read` method which checks for version < 3



-- 
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: issues-unsubscribe@trafficcontrol.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org