camaraproject / QualityOnDemand

Repository to describe, develop, document and test the QualityOnDemand API family
https://wiki.camaraproject.org/x/zwOeAQ
Apache License 2.0
41 stars 59 forks source link

Inconsistencies in the spec #80

Closed sfnuser closed 1 year ago

sfnuser commented 1 year ago

In CreateSession, 400 error response the following example is provided.

                UePortsRequired:
                  summary: uePorts is required
                  value: 
                    code: INVALID_INPUT
                    message: "Expected property is missing: uePorts"

This meant to say that uePorts is a required param. However, the CreateSession definition mentiones uePorts as optional param. Which one is correct?

Same way in the below example, it is mentioned port ranges are not allowed for uePorts. However, uePorts is of type PortSpec and there is no restriction on specifying ranges.

                UePortsRangesNotAllowed:
                  summary: Ranges at uePorts are not allowed
                  value: 
                    code: INVALID_INPUT
                    message: "Ranges not allowed: uePorts"

If these examples are not upto date, could we please remove them?

hdamker commented 1 year ago

@SfnUser yes, some of the error response examples are not consistent and make no sense within v0.8.0. We had some other examples during the review of the documentation (cf for example https://github.com/camaraproject/QualityOnDemand/pull/71#discussion_r1038167486). Luckily they are only examples and need not to be implemented. And yes, we should be more consistent here in the upcoming version.

If I'm not mistaken the two error responses you mentioned are both related to the initial thought that the uePort is needed if a public IPv4 address should to be resolved to identify an UE. In this case the uePort is needed in addition to the IPv4 address and it can't be a range. My current hope is that @eric-murray plans to clean up the error message examples in his planned PR for #34. It should then also be clearer in the spec what is expected from the API caller.

Beyond that: if an operator does not implement a certain parameter or option of the spec, the 501 code "Not Implement" might be more appropriate. As we want to use this error code I will open up a new issue to introduce it into the spec.

hdamker commented 1 year ago

Closed as discussed in call on Jan 27th.