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

Enhancements and Alignments in Errors #329

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

PedroDiez
Copy link
Collaborator

@PedroDiez PedroDiez commented Oct 30, 2024

What type of PR is this?

  • enhancement

What this PR does / why we need it:

This PR is to address agreements within #321

  1. Alignment in Mandatory Error status to be documented in API Spec YAML
  2. Alignment in Error codes regarding "API Subject" identifier (device or phoneNumber) in API Design Guidelines Section 6.2. Aligned with Update error codes and info.description template for device / phone number identifiers #324

Which issue(s) this PR fixes:

Fixes #321

Does this PR introduce a breaking change?

  • Yes
  • No

Changes in error codes to be used in API defintions

Special notes for reviewers:

Ready for review

Changelog input

Commonalities aligment regarding mandatory error `status` in API Spec YAML and common terminology for "API Subject" identifier codes. 

Additional documentation

N/A

Copy link
Collaborator

@patrice-conil patrice-conil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Comment on lines +785 to +788
**Mandatory Errors** to be **documented in CAMARA API Spec YAML** are the following:

- For event notifications flow, the ones defined in [notification-as-cloud-event.yaml](/artifacts/notification-as-cloud-event.yaml)
- For event subscriptions APIs, the ones defined in [event-subscription-template.yaml](/artifacts/camara-cloudevents/event-subscription-template.yaml)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer to include all normative requirements in the guidelines and use artifacts as illustrative files or templates.
It looks that we have respective requirements in chapter 12:

Suggested change
**Mandatory Errors** to be **documented in CAMARA API Spec YAML** are the following:
- For event notifications flow, the ones defined in [notification-as-cloud-event.yaml](/artifacts/notification-as-cloud-event.yaml)
- For event subscriptions APIs, the ones defined in [event-subscription-template.yaml](/artifacts/camara-cloudevents/event-subscription-template.yaml)
**Mandatory Errors** to be **documented in CAMARA API Spec YAML** are the following:
- For event subscriptions APIs, the ones defined in [12.1 Subscription](#error-definition-for-resource-based-explicit-subscription)
- For event notifications flow, the ones defined in [12.2 Event notification](#error-definition-for-event-notification)

Currently these points in Design Guidelines are not changed in PR #313,
I propose to introduce these changes within this PR #329 after merging PR #313

WDYT @bigludo7 @PedroDiez ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is fine, to be adressed after PR#313

- Error status 403
- Error status 429 TOO_MANY_REQUESTS (For rate limit control)

NOTE: The rest of Error status defined in section 6.1. will be documented depending on specific considerations within a given WG.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rephrasing and avoiding "WG" (it is rather Sub project or sandbox project working on API definition)

Suggested change
NOTE: The rest of Error status defined in section 6.1. will be documented depending on specific considerations within a given WG.
NOTE: The remaining Error statuses defined in section 6.1 will be documented based on specific considerations for the given API.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Spring25
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enhancements and Alignments in Errors Support & Aligment of 504 Exception in CAMARA APIs
3 participants