rimashah25 commented on code in PR #7660:
URL: https://github.com/apache/trafficcontrol/pull/7660#discussion_r1269824774


##########
lib/go-tc/deliveryservice_servers.go:
##########
@@ -168,6 +168,47 @@ type DSServerBaseV4 struct {
        DeliveryServiceCapabilities []string             `json:"-" 
db:"deliveryservice_capabilities"`
 }
 
+// DSServerBaseV5 contains the base information for a Delivery Service Server 
associated with APIv5.
+type DSServerBaseV5 = DSServerBaseV50
+
+// DSServerBaseV50 contains the base information for a Delivery Service Server 
for the latest minor version associated with APIv50.

Review Comment:
   latest `major` version not `minor`



##########
lib/go-tc/deliveryservice_servers.go:
##########
@@ -205,6 +255,15 @@ type DSServerResponseV40 struct {
 // API version 4.
 type DSServerResponseV4 = DSServerResponseV40
 
+// DSServerResponseV50 is response from Traffic Ops to a request for servers 
assigned to a Delivery Service - in  the latest minor version APIv50.

Review Comment:
   latest `major` version not `minor`



##########
lib/go-tc/deliveryservice_servers.go:
##########
@@ -180,6 +221,15 @@ type DSServer struct {
        ServerInterfaces *[]ServerInterfaceInfo `json:"interfaces" 
db:"interfaces"`
 }
 
+// DSServerV5 contains information of Delivery Service Server associated with 
APIv5.
+type DSServerV5 = DSServerV50
+
+// DSServerV50 contains information for a Delivery Service Server for the 
latest minor version associated with APIv50.

Review Comment:
   latest `major` version not `minor`



##########
docs/source/api/v5/deliveryservices_id_servers.rst:
##########
@@ -70,7 +70,7 @@ Response Structure
                :gateway:       The IPv4 or IPv6 gateway address of the server 
- applicable for the interface ``name``
                :service_address:  A boolean determining if content will be 
routed to the IP address
 
-:lastUpdated:    The time and date at which this server was last updated, in 
:ref:`non-rfc-datetime`

Review Comment:
   Did the JSON response not need an updated timestamp representing RFC3339?



##########
docs/source/api/v5/deliveryservices_id_servers_eligible.rst:
##########
@@ -76,7 +76,7 @@ Response Structure
                :gateway:       The IPv4 or IPv6 gateway address of the server 
- applicable for the interface ``name``
                :service_address:  A boolean determining if content will be 
routed to the IP address
 
-:lastUpdated:    The time and date at which this server was last updated, in 
:ref:`non-rfc-datetime`

Review Comment:
   Did the JSON response not need an updated timestamp representing RFC3339?



##########
traffic_ops/traffic_ops_golang/deliveryservice/eligible.go:
##########
@@ -96,6 +99,22 @@ func GetServersEligible(w http.ResponseWriter, r 
*http.Request) {
                api.WriteResp(w, r, v3ServerList)
                return
        }
+
+       // Based on version we load Delivery Service Eligible Server - for 
version 5 and above we use DSServerV5
+       if inf.Version.GreaterThanOrEqualTo(&api.Version{Major: 5, Minor: 0}) {
+
+               serverList := make([]tc.DSServerV5, len(servers))
+
+               for i, dss := range servers {
+                       r := time.Unix(dss.LastUpdated.Unix(), 0)

Review Comment:
   This variable name is confusing with the `http.Request` object.



##########
lib/go-tc/federation_resolver.go:
##########
@@ -72,7 +72,7 @@ type FederationResolversResponseV50 struct {
        Response []FederationResolverV5 `json:"response"`
 }
 
-// FederationResolverResponseV50 - represents struct response used for the 
latest minor version associated with APIv5.
+// FederationResolverResponseV5 - represents struct response used for the 
latest minor version associated with APIv5.

Review Comment:
   latest `major` version not `minor`



##########
traffic_ops/traffic_ops_golang/deliveryservice/servers/servers.go:
##########
@@ -724,6 +724,21 @@ func GetReadAssigned(w http.ResponseWriter, r 
*http.Request) {
                api.WriteAlertsObj(w, r, http.StatusOK, alerts, v3ServerList)
                return
        }
+
+       // Based on version we load Delivery Service Server - for version 5 and 
above we use DSServerV5

Review Comment:
   Since you are using the same logic in two places, can it not be combined in 
a function call?



-- 
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-unsubscr...@trafficcontrol.apache.org

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

Reply via email to