Release candidate preparations #62
Conversation
webrtc-registration: 0.1.3-rc.1 webrtc-call-handling: 0.1.3-rc.1 webrtc-events: 0.1.0-rc.1
stroncoso-quobis
requested review from
sushanthmavenir and
pradeepachar-mavenir
as code owners
|
@stroncoso-quobis Three points on first glance:
|
|
Regarding release numbering: For the Spring25 release the first (pre)release will be r1.1. Regarding API versioning: a v0.1.3 would be only a patch release of the existing v0.1.1 ... are you sure that there were no breaking changes at all? (e.g. by applying the Commonality guidelines?) |
| contact: | ||
| email: contact@domain.com | ||
| license: | ||
| name: Apache 2.0 | ||
| url: http://www.apache.org/licenses/LICENSE-2.0.html | ||
| version: 0.1.3-rc.1 | ||
| x-camara-commonalities: 0.4.0 | ||
|
|
There was a problem hiding this comment.
Removing trailing spaces
webrtc-registration: 0.2.0-rc.1 webrtc-call-handling: 0.2.0-rc.1 webrtc-events: 0.1.0-rc.1
stroncoso-quobis
force-pushed
the
spring25-release-candidate
branch
from
ef272da to
a35ce02
Compare
| | 2 | Design guidelines from Commonalities applied | O | M | M | M | tbd | 0.5 | | ||
| | 3 | Guidelines from ICM applied | O | M | M | M | tbd | 0.3.0 | | ||
| | 4 | API versioning convention applied | M | M | M | M | Y | SemVer 2.0.0 | | ||
| | 5 | API documentation | M | M | M | M | Y | [link](/documentation/API_documentation/webrtc-registration-API-Readiness-Checklist.md) | |
There was a problem hiding this comment.
The documentation should be inline in the YAML file info:description field, so it can be rendered with Redoc or Swagger like this:
https://editor.swagger.io/?url=https://raw.githubusercontent.com/camaraproject/WebRTC/refs/heads/main/code/API_definitions/webrtc-registration.yaml
The diagrams can be linked from Github repository (documentation folder)
As a result YAML should cover API specification and Documentation - see QoD API
| | 5 | API documentation | M | M | M | M | Y | [link](/documentation/API_documentation/webrtc-registration-API-Readiness-Checklist.md) | | |
| | 5 | API documentation | M | M | M | M | Y | inline in YAML| |
Initially it can be linked to API_documentation folder https://github.com/camaraproject/WebRTC/tree/main/documentation/API_documentation
There was a problem hiding this comment.
Cristal clear, I will try to fix it on the next commit. Thanks!
|
Hi, I think you may want to do some dedicated PR for each of the issues you listed in the beginning of this issue, and then mergen them. this would also give you a list of inputs for the changelog. Please update the name of this issue when it is ready for review as in the current state it is not clear when you will have all updates needed to pass M3/M4 for Spring25 and when/if you will cover all the checklist items for the release-candidate (M3). A release PR should only include the update of: the version(s), the Readiness checklists(s), the CHANGELOG, and the README. |
| @@ -17,9 +17,11 @@ info: | |||
| license: | |||
| name: Apache 2.0 | |||
| url: 'http://www.apache.org/licenses/LICENSE-2.0.html' | |||
| version: 0.1.3 | |||
| version: 0.2-rc.1 | |||
| x-camara-commonalities: 0.4.0 | |||
There was a problem hiding this comment.
For Spring25 meta-release, the Commonalities version shoud be 0.5, not 0.4.0
There was a problem hiding this comment.
Thanks. It is a WIP item.
Check progress at #65 PR. 0.5 comms will be covered as soon as it is merged.
| @@ -169,14 +169,14 @@ info: | |||
| license: | |||
| name: Apache 2.0 | |||
| url: https://www.apache.org/licenses/LICENSE-2.0.html | |||
| version: 0.1.0 | |||
| version: 0.1.0-rc.1 | |||
| x-camara-commonalities: 0.4.0 | |||
There was a problem hiding this comment.
For Spring25 meta-release, the Commonalities version shoud be 0.5, not 0.4.0
| contact: | ||
| email: contact@domain.com | ||
| license: | ||
| name: Apache 2.0 | ||
| url: http://www.apache.org/licenses/LICENSE-2.0.html | ||
| version: 0.2-rc.1 | ||
| x-camara-commonalities: 0.4.0 |
There was a problem hiding this comment.
For Spring25 meta-release, the Commonalities version shoud be 0.5, not 0.4.0
| | 4 | API versioning convention applied | M | M | M | M | Y | SemVer 2.0.0 | | ||
| | 5 | API documentation | M | M | M | M | Y | [link](/documentation/API_documentation/webrtc-call-handling-API-Readiness-Checklist.md) | | ||
| | 6 | User stories | O | O | O | M | N | | | ||
| | 7 | Basic API test cases & documentation | O | M | M | M | tbd | [link](/documentation/API_documentation/) | |
There was a problem hiding this comment.
Test files (.feature files) shall be put in the folder /code/Test_definitions"
| | 4 | API versioning convention applied | M | M | M | M | Y | SemVer 2.0.0 | | ||
| | 5 | API documentation | M | M | M | M | Y | [link](/documentation/API_documentation/webrtc-events-API-Readiness-Checklist.md) | | ||
| | 6 | User stories | O | O | O | M | N | | | ||
| | 7 | Basic API test cases & documentation | O | M | M | M | tbd | [link](/documentation/API_documentation/) | |
There was a problem hiding this comment.
Test files (.feature files) shall be put in the folder /code/Test_definitions"
| @@ -17,9 +17,11 @@ info: | |||
| license: | |||
| name: Apache 2.0 | |||
| url: 'http://www.apache.org/licenses/LICENSE-2.0.html' | |||
| version: 0.1.3 | |||
| version: 0.2-rc.1 | |||
| servers: | ||
| - url: '{apiRoot}/webrtc-call-handling/vwip' | ||
| - url: '{apiRoot}/webrtc-call-handling/v0.2' |
There was a problem hiding this comment.
the versions here should be: v0.2rc1
| x-camara-commonalities: 0.4.0 | ||
|
|
||
| externalDocs: | ||
| description: Product documentation at CAMARA | ||
| url: https://github.com/camaraproject/ | ||
| servers: | ||
| - url: "{apiRoot}/webrtc-events/vwip" | ||
| - url: "{apiRoot}/webrtc-events/v0.1" |
There was a problem hiding this comment.
the url version should be: v0.1rc1
| contact: | ||
| email: contact@domain.com | ||
| license: | ||
| name: Apache 2.0 | ||
| url: http://www.apache.org/licenses/LICENSE-2.0.html | ||
| version: 0.2-rc.1 |
There was a problem hiding this comment.
the version should be: 0.2.0-rc.1
| servers: | ||
| - url: '{apiRoot}/webrtc-registration/vwip' | ||
| - url: '{apiRoot}/webrtc-registration/v0.2' |
There was a problem hiding this comment.
the url version shoudl be v0.2rc1
There was a problem hiding this comment.
Comments on the terminology used in the API OAS file:
Too much Telco stuff that we should avoid in CAMARA APIs: we should design the API with
- user-friendly terms in the info.description documentation
- user friendly names for schemas
- not refer to any network equipment (e.g. webRTC GW), just use "network" or drop it alltogether.
line 7 and rest of spec(s): do we need the "vvoip" part in the "vvoipSessionId", or could we just use "sessionId" ? "vvoip" is a "Telco term".
If you don't want to change, the term "vvoip" should be clearly explained before using it, but I recommend to drop it
The name of the API (webrtc-call-handling" already includes the information about the type of sessions (sort-of) handled by this API, although I am still not so happy with the "webrtc" part.
The term "webrtc" is also Telco stuff: The API name could be "one-to-one-calling" instead ? you use that term in e.g. the tags. Unless you want to evolve this API later to support more than 2 participants which should then be foreseen.in the API design.
An alternative could be to use "person-to-person-calling" as it does not say how many persons and would allow for evolution of the API to multiple parties.
line 9: replace RACM with new name
line 41/42 and rest of file: the term "hdr" is not explained. we should avoid Telco abbreviations in the CAMARA APIs. Can you change it to a more developer friendly term ?
and related:
lines 331 hdrClientId and 340 hdrTransactioId: are these really informations that need to be exposed to the applications ? if so
- these parameters should be explained in the API documentation in the info.description section of the OAS yaml.
- they should be replaced by more user friendly names, or, at least, drop the "hdr" part
line 266: WrtcSDPDescriptor - same point about Telco stuff. we should designe the API in user-friendly terms in the info.description documentation and find a user friendly names for schemas - you really want to avoid a developer having to read the SDP RFC ...
If this is the attribute in the API POST allows to choose VOICE or VIDEO for the call to be created, could you make that the more user-friendly ? e.g. with a CallType parameter with a schema with an ENUM ? Maybe have a look at the APIs created by some of the CPaaS players for their Voice and Video calling ?
There was a problem hiding this comment.
Hi @tanjadegroot ,
Thanks for the review. Let's try to add some responses, and we will see how to fix them:
Replace WebRTCGW by network
Sounds good to me, I will take a look over it. All documentation needs to be reviewed.
replace vvoipSessionId as "vvoip" is a Telco term
vvoipSessionId and racmSessionId are used for different kind of "sessions", one is the media session itself, the other one is the active registration of the user/device on the network. I think that is better to keep some difference between them.
The term "webrtc" is also Telco stuff
In my humble opinion, I disagree, "webrtc" is a developer or technology term. It comes from the WebRTC API , and it is widely used on the real-time developers landscape, WHIP/WHEP is an example of protocol that uses that techonlogy and most of users are familiar with the SDP terminology. At least when dealing with "calls".
call-regsitration, call-handling and call-events can be possible names, but "webrtc" does not look to me as a "Telco stuf", is more a "CPaaS stuff".
Unless you want to evolve this API later to support more than 2 participants
I will really look for this, and yes, trying to be inspired by the most popular CPaaS, in order to bring easy transition from private CPaaS to Telco network systems.
line 9: replace RACM with new name
Fair enough :) RACM stands for Registration And Connectivity Manager, it could be simplier, like "registration service" or "webrtc registration", we will try to came up with something new.
the term "hdr" is not explained
hdrClientId and 340 hdrTransactioId: are these really informations that need to be exposed to the applications ?
These terms probably came from previous specs that weren't adapted to the current usage of x-correlator and deviceId. We will check them. For sure hdr means header and we will think on something to change it.
line 266: WrtcSDPDescriptor [...] you really want to avoid a developer having to read the SDP RFC ...
Yes, we need to avoid the SDP direct manipulation, it must not be a requirement. But is needed to provide a protocol to share the media descriptor of each side of the Real-Time communication.
As said before, is my belief that SDP is really common nowadays for RTC developers. That I consider the target of the API. What we must include on the documentation, is to provide clear instructions about how to obtain this SDP. Maybe some reference to RTCSessionDescription or other mechanisms.
We will take a look to this feedback.
If this is the attribute in the API POST allows to choose VOICE or VIDEO for the call to be created, could you make that the more user-friendly ? e.g. with a CallType parameter with a schema with an ENUM ? Maybe have a look at the APIs created by some of the CPaaS players for their Voice and Video calling ?
There are a lot of room for improvement, this audio/video request is one of this possible improvements, there's no specification for that on the API. You can respond with what you want, based on the SDP description.
For future releases, we need to review what is being done out there and try to get improvements. But this basic usage, within the CAMARA initiative will ease the developer pain for all the different CPaaS and GWs on the market.
We will discuss your feedback and create issues for the possible changes agreed.
Thanks a lot for your feedback.
Regards,
There was a problem hiding this comment.
Hi @stroncoso-quobis? I am happy to stand corrected on my view that WebRTC is a Telco term. If this is what is being used the the CPaaS context, then it makes sense to adopt related terminology. Maybe this is also due the abbreviations :-)
However looking at terminology on your link (https://developer.mozilla.org/en-US/docs/Web/API/WebRTC_API#webrtc_reference),
I think it would still be important to find more "human" terms, e.g. inspired by some of the terms used on that page but without the "RTC" part. For example having
- api name: "real-time-communications"
- url: "{apiRoot}/real-time-communications/vx"
- api: "/sessions"
- parameter: sessionDescription
- etc.
That would
- make the link with WebRTC for those who know it (and documented in the description)
- avoid abbreviations in the APIs and parameters
- make it more easily readable
- is generic enough to allow evolution to more features later
you may then be able to reach yet more developers :-)
Up to the team to discuss for sure.
There was a problem hiding this comment.
I have a number of comments on the design (OAS structure) of the API OAS file:
- instead of using the same schema VvoipSessionInformation for mostly all requests and responses, it would be preferable (for inter-operability reasons) to have separate createRequestBody with dedicated structures and with required attributes list, instead of having it it in the text only.
- tags: usually these are the space-separated uppercase variant of the API name, not something completely different (not helpful for trouble shooting)
- In paths section:
- the summary and the description should not be the same. there should be more info in the description.
- operationId: would be derived from the operation, e.g. createSession (for POST), listSessions, (for GET) getSession, deleteSession, etc. (no "DetailsById" needed"), etc
- parameters shall be user friendly and explained in the description (see also other comment)
- examples: it is recommended to user-frienldy names for examples. e.g. exMT180 is rahter cryptic.
line 154: the operationId of the PUT method would be ""putSessionStatus" or modifySessionStatus" instead of ""postSessionStatus"
line 292: for ENUM values, most APIs use UPPER_SNAKE_CASE for the values (but this is today not in the API guidelines
line 325: "Address" - this seems to be the same type as the CAMARA_common "PhoneNumber". suggest to use that one instead
line 348: "pathParamVvoipSessionId": could this just be called "sessionId" or some other simplified name ?
for security reasons:
- schema type patterns shall be provided wherever possible, especially for strings
- maxlen of strings should be provided
There was a problem hiding this comment.
I assume similar comments as the above 2 would apply to the other API OAS files - I'll let you cross check this.
|
Suggestion from Release mgmt team: we consider that Spring25 meta-release is too early given the current state of the API. |
Can we discuss this recommendation?
During today's meeting, deadlines were commented, and we strongly believe that we can still achieve the deadline. Please, in name of all the team, reconsider the suggestion and give us some deadline to deliver doc & testing. Best regards, |
|
@stroncoso-quobis Hi Santiago and team, of course we will be happy if you can deliver the API assets. FYI the following are the next dates:
Note: the M4 date (final public release) is Feb 28th: between M3 and M4 only bug fixes on the API and documentation updates are expected. Hope you will be able to get there ! |
What type of PR is this?
Add one of the following kinds:
What this PR does / why we need it:
Which issue(s) this PR fixes:
Fixes #50
Fixes #51
Fixes #61
Fixes #60
Fixes #58
Special notes for reviewers:
This PR intend to solve any formality and documentation requirements to achieve public release status within the repository and APIs: webrtc-registration, webrtc-call-handling, and webrtc-events. No functionality changes are intended.
Changelog input
Additional documentation