zrhoffman commented on code in PR #7405:
URL: https://github.com/apache/trafficcontrol/pull/7405#discussion_r1152415417


##########
lib/go-tc/deliveryservices.go:
##########
@@ -49,6 +49,14 @@ const MaxRangeSliceBlockSize = 33554432
 // values of a Delivery Service's 'Active' property (v3.0+).
 type DeliveryServiceActiveState string
 
+// These names of URL query string parameters are not allowed to be in a
+// Delivery Service's "ConsistentHashQueryParams" set, because they collide 
with
+// query string parameters reserved for use by Traffic Router.
+const (
+       ReservedConsistentHashingQueryParameterFormat = "format"
+       ReservedConsistentHashingQueryParameterTRRED  = "trred"

Review Comment:
   It's true that reserved consistent hashing query parameters includes these, 
but that doesn't describe what they actually are. For reuse, should these be 
grouped with other Traffic Router-related constants?
   
   For example, this is a header, not a GET variable, but maybe GET variables 
could go somewhere nearby:
   
   
https://github.com/apache/trafficcontrol/blob/7741530c494d797758d226885abee3ed7feda7be/lib/go-tc/traffic_router.go#L55-L57



##########
lib/go-tc/deliveryservices.go:
##########
@@ -49,6 +49,14 @@ const MaxRangeSliceBlockSize = 33554432
 // values of a Delivery Service's 'Active' property (v3.0+).
 type DeliveryServiceActiveState string
 
+// These names of URL query string parameters are not allowed to be in a
+// Delivery Service's "ConsistentHashQueryParams" set, because they collide 
with
+// query string parameters reserved for use by Traffic Router.
+const (
+       ReservedConsistentHashingQueryParameterFormat = "format"
+       ReservedConsistentHashingQueryParameterTRRED  = "trred"

Review Comment:
   Should probably include `"fakeClientIpAddress"`, too
   
   
https://github.com/apache/trafficcontrol/blob/7741530c494d797758d226885abee3ed7feda7be/traffic_router/core/src/main/java/org/apache/traffic_control/traffic_router/core/request/HTTPRequest.java#L30



-- 
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