-
Notifications
You must be signed in to change notification settings - Fork 28
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
base: main
Are you sure you want to change the base?
Enhancements and Alignments in Errors #329
Conversation
…o Enhancements_and_Alignments_in_Errors
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
**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) |
There was a problem hiding this comment.
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:
**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 ?
There was a problem hiding this comment.
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. |
There was a problem hiding this comment.
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)
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. |
What type of PR is this?
What this PR does / why we need it:
This PR is to address agreements within #321
status
to be documented in API Spec YAMLWhich issue(s) this PR fixes:
Fixes #321
Does this PR introduce a breaking change?
Changes in error codes to be used in API defintions
Special notes for reviewers:
Ready for review
Changelog input
Additional documentation
N/A