camaraproject / QualityOnDemand

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

Default duration should be only an example #115

Closed hdamker closed 1 year ago

hdamker commented 1 year ago

Issue

Within API documentation the default session duration of 24 hours is only given as an example:

but in the API YAML file it is fixed:

duration:
          type: integer
          example: 86400
          description: |
            Session duration in seconds. Maximal value of 24 hours is used if not set.
            After session has expired the client will receive QOS_STATUS_CHANGED event with 
             - qosStatus is 'UNAVAILABLE', and, 
             - statusInfo is 'DURATION_EXPIRED'. 
            See notification callback.
          format: int32
          minimum: 1
          maximum: 86400
          default: 86400

Proposal:

Omit "Maximal value of 24 hours is used if not set" and the default: 86400 line from the general YAML, 24 hours is then still the maximum and implicit default value, if there is not other default value defined by the network operator. A network operator can set a specific default for their network enviroment. A PR should also make the documentation more precise and encourage developers to set the duration if they expect a certain duration.

Background:

We would like to set a short default duration in certain networks (using environment variable), e.g. in case of test environments (as a mitigation to issue described in #101). Also a default of 24 hrs might be too long from business perspective in production networks.

jlurien commented 1 year ago

Sounds reasonable to me. As you say the maximum implies an upper limit, so no implementation should allow a higher value, but we may allow different policies regarding the default. In any case, the client can always request for a value explicitly, and it will be informed of the applied value in the response.

hdamker commented 1 year ago

We propose to not fix the issue. For test/lab deployments we have internal (not schema conform) solution to configure the default value. Leaving out the default value in the API spec would make the description incomplete.

A an alternative (correct) solution would be to make the duration paramater mandatory, but that would be a different issue.

hdamker commented 1 year ago

Won't fix.