search

camaraproject / EdgeCloud

Repository to describe, develop, document and test the EdgeCloud API family
Apache License 2.0
16 stars 43 forks source link

Update EdgeCloud_LcM.yaml #165

Closed gunjald closed 7 months ago

gunjald commented 8 months ago

Updated /app POST method 201 response code description and added the "location" header to return the URI of the newly created application resource

sergiofranciscoortiz commented 8 months ago

The suggestion of adding a location parameter response for the app it does not seem appropriate, as in the conception on this API, the app object includes "how" the application is instantiated, but not "where". It is in the appInstance object where it is included an URI and an edgeCloudNodeName in the AppInstanceInfo, which reflects the location of a running instance of the application.

gunjald commented 8 months ago

The suggestion of adding a location parameter response for the app it does not seem appropriate, as in the conception on this API, the app object includes "how" the application is instantiated, but not "where". It is in the appInstance object where it is included an URI and an edgeCloudNodeName in the AppInstanceInfo, which reflects the location of a running instance of the application.

I suppose the "Location" header is a standard header and it has no binding to the business context attributes e.g. customer location etc. It is more to convey the URI of the new resource created as outcome of executing the POST call at the server and not to carry any kind of domain details. This URI can later be used to query or update the newly created resource.

sergiofranciscoortiz commented 8 months ago

Thanks for the clarification, so the proposed change is more to follow standard HTTP 201 responses inluding a "Location" header, but with no implications in the business logic. With the response of appId, it was covered the need to be used for query or updated of the created app, but returning a Location header may be more complete. I see that in Traffic Influence API, it is also included in the response of creating a TrafficInfluence object with a 201 response, although it does not seem to be widely used in other CAMARA APIs, and no explicit reference in https://github.com/camaraproject/Commonalities/blob/main/documentation/API-design-guidelines.md#32-http-response-codes, although implicitely refers to https://datatracker.ietf.org/doc/html/rfc7231#section-6.5.1, where the use of a header Location is advised. I would accept your proposed change as a best practice, but I would like to know the opinion form others. @FabrizioMoggio since you included Location header in TI API, what do you think?

FabrizioMoggio commented 7 months ago

I agree with @gunjald view. In TI API we use the "location" header to return the URI of the newly created TI resource, anyway that pattern can be used to return any object as defined here https://datatracker.ietf.org/doc/html/rfc7231#section-7.1.2

sergiofranciscoortiz commented 7 months ago

Taking into account Fabrizio's comment (thanks), I will go ahead an merge this pull request.