ocket8888 commented on a change in pull request #5094:
URL: https://github.com/apache/trafficcontrol/pull/5094#discussion_r499915105
##########
File path: traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go
##########
@@ -266,6 +266,14 @@ func createV30(w http.ResponseWriter, r *http.Request, inf
*api.APIInfo, ds tc.D
deepCachingType = ds.DeepCachingType.String() // necessary,
because DeepCachingType's default needs to insert the string, not "", and Query
doesn't call .String().
}
+ if ds.Topology != nil {
+ if ok, err := dbhelpers.TopologyExistsString(tx, *ds.Topology);
err != nil {
+ return nil, http.StatusInternalServerError, nil,
fmt.Errorf("checking topology existence: %v", err)
+ } else if !ok {
+ return nil, http.StatusConflict, fmt.Errorf("no such
Topology '%s'", *ds.Topology), nil
Review comment:
The example with the PUT request is just that - an example. Conditional
requests aren't the only use allowed for `409 Conflict` - however,
[RFC7231](https://tools.ietf.org/html/rfc7231#section-6.5.8) (which obsoletes
2616) *does* state that it should be used when the conflicted state is that of
the "*target* resource", which makes it feel inappropriate. The wording on
[MDN's `409 Conflict`
page](https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/409) is a bit
more permissive:
> "The HTTP `409 Conflict` response status code indicates a request conflict
with current state of the server."
which is very different that the state of the target resource. That's where
my definition of 409 was coming from, but it seems inaccurate.
So, what, 400 then?
----------------------------------------------------------------
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]
