Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Should errors 405, 415, 500, 501, 502, 503, 504 be listed in CAMARA_common.yaml #355

Open
rartych opened this issue Dec 9, 2024 · 4 comments
Labels
correction correction in documentation

Comments

@rartych
Copy link
Collaborator

rartych commented Dec 9, 2024

Problem description
It was agreed in #321 not to include some 40X and 50X errors in API definitions.
Errors 405, 415, 500, 501, 502, 503, 504 are present in CAMARA_common.yaml

Expected behavior
Remove definitions from CAMARA_common.yaml

Alternative solution
Clarification if errors stay in CAMARA_common.yaml.

Additional context

@PedroDiez
Copy link
Collaborator

Think we have two options:

  • Option 1: Remove 405, 415, 500, 501, 502, 503, 504 from CAMARA_common.yaml

In https://github.com/camaraproject/Commonalities/blob/main/documentation/API-design-guidelines.md#6-error-responses:

My point here would be to also remove 406.

PROS:

  • Not havind them in CAMARA_common.yaml may help in order to not include in API SPECs

CONS:

  • What happens in the opposite way? When we have an API that really need to document any of the above errors even it is not mandatory as already agreed. And such an scenario will be documented in ATP as well. We will reach then the conclusion "we have to document back in CAMARA_common.yaml"?

I am fine with this option, but at the same time being transparent with the implications it may have.

  • Option 2: Keep them in CAMARA_Common.yaml, so the error structure is documented for reference in case any API design needs it.

Modify in API-design-guidelines.md#61-standardized-use-of-camara-error-responses

As per below (some wording like this):

Mandatory Errors to be documented in CAMARA API Spec YAML are the following:

  • For event subscriptions APIs, the ones defined in 12.1 Subscription
  • For event notifications flow, the ones defined in 12.2 Event notification
  • For the rest of APIs:
    • Error status 401
    • Error status 403
    • Error status 429 TOO_MANY_REQUESTS (For rate limit control)

HERE BEGINS THE UPDATE

NOTE:

  • Not Mandatory error statuses will be documented *in an API Specification* based on specific considerations for the given API. Current Guidelines:

Error statuses 400, 404, 409, 422 depends on API design and functionality involved
Error statuses 405, 406, 410, 412, 415, 5xx are NOT document. They will be ONLY documented in a given API specification if there is relevant Use Case related and therefore it MUST be considered with their Testing.

WDYT @rartych, @bigludo7, @eric-murray, @shilpa-padgaonkar, @patrice-conil, @jlurien

@PedroDiez
Copy link
Collaborator

I am fine with both options. But honestly i prefer Option 2

@eric-murray
Copy link
Collaborator

I would prefer Option 2 as well, but why not add a comment in CAMARA_common.yaml for each error schema to say whether it is mandatory or optional to include it in the API yaml?

@bigludo7
Copy link
Collaborator

I'm fine with the option 2.

I'm also in favor to add your proposal explicitly in the guidelines (I have copied to be sure about it because the formatting is confusing)


(...)

Note:
Not Mandatory error statuses will be documented in an API Specification based on specific considerations for the given API. Current Guidelines:

  • Error statuses 400, 404, 409, 422 depends on API design and functionality involved
  • Error statuses 405, 406, 410, 412, 415, 5xx are NOT documented by default in API specification. They will be ONLY documented in a given API specification if there is relevant Use Case related and therefore it MUST be considered with their Testing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
correction correction in documentation
Projects
None yet
Development

No branches or pull requests

4 participants