Update API specification to follow common guidelines#25
Update API specification to follow common guidelines#25DanXu-ChinaTelecom wants to merge 1 commit intomainfrom
Conversation
|
@DanXu-ChinaTelecom there are some points which are not yet in line with the CAMARA Design Guidelines. General:
Some hints about further alignment potential in the following review comments. Hope they are helpful. |
hdamker
left a comment
hdamker
left a comment
There was a problem hiding this comment.
Some hints to improve the alignment with the guidelines in Commonalities, not meant as a complete review.
| @@ -0,0 +1,936 @@ | |||
| openapi: "3.0.3" | |||
| info: | |||
| title: Site to Cloud VPN API | |||
There was a problem hiding this comment.
Title shouldn't contain "API"
| title: Site to Cloud VPN API | |
| title: Site to Cloud VPN |
| description: | | ||
| ##### Before starting to use the API, the developer needs to know about the below specified details : | ||
| * Site to Cloud VPN service endpoint The URL pointing to the RESTful resource of the Site to Cloud VPN API. | ||
| * Authentication Security access keys such as OAuth 2.0 client credentials used by client applications to invoke the Site to Cloud VPN API. |
There was a problem hiding this comment.
Please consider the documentation within Identity and Consent Management. It is not "such as" but clearly defined to be Client Credentials.
| * Authentication Security access keys such as OAuth 2.0 client credentials used by client applications to invoke the Site to Cloud VPN API. | |
| * Authentication Security access keys such as OAuth 2.0 client credentials used by client applications to invoke the Site to Cloud VPN API. |
|
|
||
| NOTE- The private physical link between CE and cloud PE is default deployed for one click calling, otherwise it is required to install the physical link, which is out of the scope of this API. | ||
|
|
||
| version: '0.1' |
There was a problem hiding this comment.
Version is wip until the repository is released, see release process in Release Management and API versioning in chapter 5 of API Design Guidelines in Commonalities:
| version: '0.1' | |
| version: wip | |
| servers: | ||
| - url: https://localhost:8080/webapi | ||
| description: develop server |
There was a problem hiding this comment.
Please look for correct format of servers object in Commonalities, example in
Please define also the API name here, e.g. site-to-cloud-vpn, don't abbreviate it (it should correspond with the title.
| X-Correlator: | ||
| description: When the API Consumer includes the "X-Correlator" header in the request, the API provider must include it in the response with the same value that was used in the request | ||
| schema: | ||
| type: integer |
There was a problem hiding this comment.
- x-correlator instead of X-Correlator
- format is string, not integer
- there is a schema defined within Components section
| rent: | ||
| type: integer | ||
| description: "VPN monthly rental " |
There was a problem hiding this comment.
Does an integer without a currency make sense here?
| isProtected: | ||
| type: bool | ||
| description: Is Site to Cloud VPN protected | ||
| CEType: |
There was a problem hiding this comment.
Here and for many other parameters:
- avoid abbreviations
- lowerCamelCase
| content: | ||
| application/json: | ||
| schema: | ||
| type: object |
There was a problem hiding this comment.
This object has lot in common with S2CVPNOrder. Maybe it would make sense to define a base object in Components and derive the other object type from it?
|
@hdamker thanks a lot for your careful review, we'll check all the comments and handle the related issues. |
|
/easycla |
2 participants
What type of PR is this?
Add one of the following kinds:
What this PR does / why we need it:
Check design guidelines from Commonalities applied
Which issue(s) this PR fixes:
Fixes ##24
Special notes for reviewers:
Changelog input
Additional documentation
This section can be blank.