Fix/release m4 review #71
Conversation
- registration: description review, extDocs URL, tags review, endpoint description and operation ids, removed RACM standalone term - call-handling: extDocs URL, endpoint descriptions, tags review, operationId, example names review - events: extDocs URL, tags review, endpoint description review, removed RACM standalone term
|
Hi team, Inform about a couple of changes pending, that are already in progress:
And pending to analyze requirements for webrtc-events (subscription API) due M4 public release of commonalities. As soon as they are ready, I will mark the PR as ready to merge. Best regards, |
- Replaced "racmSessionId" by "regSessionId" - Replaced "vvoipSessionId" by "mediaSessionId" - Replaced variables used on feature testing
|
All points identified at #70 should be fixed. Some extra corrections included. We replaced some terms like "racm" and "vvoip" to keep a clearer syntax. Also we included Looking forward for reviews and comments |
|
@stroncoso-quobis There are two options:
Given the short time until Friday I recommend to go for option 1. cc: @tanjadegroot |
- UTC timestamp description on AccessExpireUtc - Sink uri update - 409 response review - SinckCredentials removed from responses
|
Hi team, @hdamker , @tanjadegroot , @pradeepachar-mavenir Proceeding with requested changes, yes, as suggested by Herbert, we are pointing to M4 release. So summarizing changes from latest comment:
Last commit for trailing spaces 🤦 Hope everything is ok. Waiting for your reviews Best regards, |
CHANGELOG.md
Outdated
|
|
||
| Changelog since r1.1 | ||
|
|
||
| * Full Changelog with the list of PRs and contributors: https://github.com/camaraproject/DeviceLocation/compare/r1.1...r1.2 |
There was a problem hiding this comment.
For a public release, the changelog should cover all changes since the last pubic release (since v0.1.1)
README.md
Outdated
| * Create subscriptions to receive telco call events using CloudEvents | ||
| * Make calls from the endpoint (client) side establishing a realiable and media WebSocket channel to exchange secure media. | ||
| * Create subscriptions to receive telco call events using CloudEvents: Ability to follow call estalishment process and to receive new incoming calls. | ||
| * Make and receive calls from the endpoint (client) side establishing a realiable media WebSocket channel to exchange secure media. | ||
| * Describe, develop, document and test the APIs (with 1-2 Telcos) | ||
| * Started: September 2023 | ||
| * Location: virtually |
| @@ -200,13 +199,13 @@ paths: | |||
| - `org.camaraproject.webrtc-events.v0.session-status`: updates on specific call | |||
|
|
|||
| To subscribe and receive notifications about new calls, a | |||
| `session-invitation` subscription is required using the `racmSessionId` | |||
| `session-invitation` subscription is required using the `regSessionId` | |||
| obtained from webrtc-registration API. | |||
|
|
|||
| Once you create a new voice-video session or receive an | |||
| new call event, you can subscribe to `session-status` to receive | |||
| updates about the voice-video session in progress using the | |||
There was a problem hiding this comment.
| updates about the voice-video session in progress using the | |
| updates about the media session in progress using the |
| type: string | ||
| example: xsmcaum3z4zw4l0cu4w115m0 | ||
| description: |- | ||
| The RACM session ID created by WebRTC Gateway. | ||
| Obtained via RACM API. | ||
| The resgitration session ID created by WebRTC Gateway. |
There was a problem hiding this comment.
| The resgitration session ID created by WebRTC Gateway. | |
| The registration session ID created by the network. |
| type: string | ||
| description: The VVOIP session ID created by WebRTC Gateway. The vvoipSessionId shall not be included in POST requests by the client, but must be included in the notifications from the WebRTC GW to client. | ||
| description: The VVOIP session ID created by WebRTC Gateway. The mediaSessionId shall not be included in POST requests by the client, but must be included in the notifications from the WebRTC GW to client. |
|
|
||
| Identified by for `vvoipSessionId`, and validated via a `racmSessionId` | ||
| Identified by for `mediaSessionId`, and validated via a `regSessionId` | ||
| the endpoint is able to interact with its own voice-video session. |
| @@ -1393,7 +1412,7 @@ components: | |||
| A new WebRTC session is requested (i.e. incoming call). This event | |||
There was a problem hiding this comment.
WebRTC --> media
comment applicable to the whole file
Co-authored-by: Tanja de Groot <87864067+tanjadegroot@users.noreply.github.com>
There was a problem hiding this comment.
According to the guidelines, the filename should have the extension "-subscriptions" ,so the name should be "webrtc-event-subscriptions.yaml"
There was a problem hiding this comment.
This file seems to be specifically designed for asynchronous event handling. However, it does not seem to be used as no test definition nor Readiness checklist are provided.
I think this file can be deleted (as the webrtc-events YAML already covers the asynchronous cases as well).
If not the case, then let me know as I skipped it for the review right now.
There was a problem hiding this comment.
line 11: "... video sessions, you device (or endpoint) need to have a valid registration on"
- Add after "sessions"", a.k.a media sessions".
- your --> your
- endpoint --> notification endpoint
- need --> needs
line 12: "the network. This API provide that feature, and allow users to create audio and
video sessions using the telco network."
- provide --> provides
- allow --> allows"
- remove "telco" - also from other occurences of "telco network" in the rest of the file
documentation/API_documentation/webrtc-events-API-Readiness-Checklist.md
Outdated
Show resolved
Hide resolved
documentation/API_documentation/webrtc-registration-API-Readiness-Checklist.md
Outdated
Show resolved
Hide resolved
documentation/API_documentation/webrtc-registration-API-Readiness-Checklist.md
Outdated
Show resolved
Hide resolved
documentation/API_documentation/webrtc-registration-API-Readiness-Checklist.md
Outdated
Show resolved
Hide resolved
documentation/API_documentation/webrtc-call-handling-API-Readiness-Checklist.md
Outdated
Show resolved
Hide resolved
- suggestions about redaction and spelling - remove rc.1 sufixes - replace references to r1.1 with r1.2 Co-authored-by: Tanja de Groot <87864067+tanjadegroot@users.noreply.github.com>
There was a problem hiding this comment.
Many thanks for the additional updates.
Can you please still clarify on the access token part (see #71 (comment))
Also there seem to be some errors (trailing spaces) to be fixed in webrtc-registration.yaml
|
|
||
| * a `registrationId` to manage a registration later | ||
| * a `clientId` (in the `connectivityInformation`) that must be subsequently used in all webrtc service API calls to the network for that end user. | ||
| session before enabling or activating any service. In order to create audio and |
There was a problem hiding this comment.
there seems to be a partial sentence here ?
|
|
||
| * a `registrationId` to manage a registration later | ||
| * a `clientId` (in the `connectivityInformation`) that must be subsequently used in all webrtc service API calls to the network for that end user. | ||
| session before enabling or activating any service. In order to create audio and | ||
| video sessions, you device (or endpoint) need to have a valid registration on |
There was a problem hiding this comment.
| video sessions, you device (or endpoint) need to have a valid registration on | |
| video sessions, your device (or endpoint) needs to have a valid registration on |
|
|
||
| * a `registrationId` to manage a registration later | ||
| * a `clientId` (in the `connectivityInformation`) that must be subsequently used in all webrtc service API calls to the network for that end user. | ||
| session before enabling or activating any service. In order to create audio and | ||
| video sessions, you device (or endpoint) need to have a valid registration on | ||
| the network. This API provide that feature, and allow users to create audio and |
There was a problem hiding this comment.
| the network. This API provide that feature, and allow users to create audio and | |
| the network. This API provides that feature, and allows users to create audio and |
|
|
||
| * a `registrationId` to manage a registration later | ||
| * a `clientId` (in the `connectivityInformation`) that must be subsequently used in all webrtc service API calls to the network for that end user. | ||
| session before enabling or activating any service. In order to create audio and | ||
| video sessions, you device (or endpoint) need to have a valid registration on | ||
| the network. This API provide that feature, and allow users to create audio and | ||
| video sessions using the telco network. |
There was a problem hiding this comment.
| video sessions using the telco network. | |
| video sessions using the network. |
| @@ -115,7 +119,7 @@ paths: | |||
| - Registration | |||
| summary: Create a registration | |||
| description: |- | |||
| Endpoint used to create a new registration on the network. This action | |||
| API used to create a new registration of a device with the network identified by a registrationId (in UUID format). | |||
| creates a valid communications session on the network system identified | |||
There was a problem hiding this comment.
| creates a valid communications session on the network system identified | |
| creates a valid communications session on the network, identified |
| @@ -152,7 +156,7 @@ paths: | |||
| - Registration | |||
| summary: Refresh access token and network registration. | |||
| description: | | |||
| API Endpoint used to refresh the resgitration on the network. Sharing a | |||
| API Endpoint used to refresh the registration on the network. Sharing a | |||
| new AccessToken with network if it corresponds. The AccessToken | |||
There was a problem hiding this comment.
| new AccessToken with network if it corresponds. The AccessToken | |
| new AccessToken with the network if it corresponds. The AccessToken |
| @@ -115,7 +119,7 @@ paths: | |||
| - Registration | |||
| summary: Create a registration | |||
| description: |- | |||
| Endpoint used to create a new registration on the network. This action | |||
| API used to create a new registration of a device with the network identified by a registrationId (in UUID format). | |||
| creates a valid communications session on the network system identified | |||
| by a `regSessionId'. This unique ID is used for each action where the | |||
| endpoint has been involved in connecting to the network. | |||
There was a problem hiding this comment.
This is a bit hard to read: "This unique ID is used for each action where the endpoint has been involved in connecting to the network."
suggest to update to
"This unique registration ID is used in each API call handling sessions on the network for the device."
|
OK, let's try to summarize all changes for this M4 review at 0369fde
Find a readable documentation at: Changes pending for today's discussion at working group meeting:
Regards, |
- Removed rc.x - Replaced "vvoip" references for "media" - Replaced "webrtc gw" references for "network" - Removed references to NotificationChannel - Improved terms and definitions - Improved deviceId descriptions - Removed access token reference on PUT registration - CHANGELOG reviewed - Trailing spaces and spelling errors - Renamed webrtc-events.yaml to webrtc-events-subscription.yaml
stroncoso-quobis
force-pushed
the
fix/release-m4-review
branch
from
e367dd4 to
0369fde
Compare
- Replaced "msisdn" by "phoneNumber" at registration response
- Review of {{field}} 403 responses (auth context)
- Replace of hdrClientId by hdrRegistrationId creating media sessions
- Updated changes at testing files
- Remove trailing spaces
|
Hi all, Based on today's working group meeting notes, we pushed commits 83b95ae and 3458ff3
On these commits we fullfil all pending requirements pointed on the release review:
All is ready for review. @pradeepachar-mavenir , please confirm changes Best regards 👋 |
There was a problem hiding this comment.
Looks good !
For your consideration: Please check some english and terminology consistency proposals below and 2 questions (also terminology related) and a clarification on the hdrRegistrationId
I approve these changes. Many thanks for the hard work !
/LGTM from Release Management
|
|
||
| ## Scope | ||
| * Service APIs for “WebRTC” (see APIBacklog.md) | ||
| * It provides the customer with the ability to: | ||
| * It provides the API consumer with the ability to: | ||
| * Add real-time communication capabilities to their applications like video, voice, and generic data. | ||
| * Enable a web endpoint to register for telco services (IMS) through a WebRTC Gateway using Oauth 2.0 and the Operator provided Identity Provider (IDP) |
There was a problem hiding this comment.
| * Enable a web endpoint to register for telco services (IMS) through a WebRTC Gateway using Oauth 2.0 and the Operator provided Identity Provider (IDP) | |
| * Enable an API consumer to register for communication services through a network using Oauth 2.0 and OIDC. |
| * Add real-time communication capabilities to their applications like video, voice, and generic data. | ||
| * Enable a web endpoint to register for telco services (IMS) through a WebRTC Gateway using Oauth 2.0 and the Operator provided Identity Provider (IDP) | ||
| * Create subscriptions to receive telco call events using CloudEvents | ||
| * Make calls from the endpoint (client) side establishing a realiable and media WebSocket channel to exchange secure media. | ||
| * Create subscriptions to receive telco call events using CloudEvents: Ability to follow call estalishment process and to receive new incoming calls. |
There was a problem hiding this comment.
| * Create subscriptions to receive telco call events using CloudEvents: Ability to follow call estalishment process and to receive new incoming calls. | |
| * Create subscriptions to receive call notifications using CloudEvents: Ability to follow the call establishment process and to receive new incoming calls. |
| * Create subscriptions to receive telco call events using CloudEvents | ||
| * Make calls from the endpoint (client) side establishing a realiable and media WebSocket channel to exchange secure media. | ||
| * Create subscriptions to receive telco call events using CloudEvents: Ability to follow call estalishment process and to receive new incoming calls. | ||
| * Make and receive calls from the endpoint (client) side establishing a realiable media WebSocket channel to exchange secure media. |
There was a problem hiding this comment.
| * Make and receive calls from the endpoint (client) side establishing a realiable media WebSocket channel to exchange secure media. | |
| * Make and receive calls from the API consumer (client) side establishing a realiable media session to securely exchange media. |
| @@ -9,16 +9,16 @@ info: | |||
|
|
|||
| To handle audio/video sessions, what we usually known as *calls*, the network | |||
| requires to identify both involved parties: caller, the origin, and the callee, | |||
| the destination. Once identified the endpoints required to be connected, then | |||
| the destination. Once identified the involved communication endpoints, then | |||
There was a problem hiding this comment.
| the destination. Once identified the involved communication endpoints, then | |
| the destination. Once the involved communication endpoints are identified, |
There was a problem hiding this comment.
in this section, would "communication endpoints" or "endpoints" be "devices" ?
| @@ -9,16 +9,16 @@ info: | |||
|
|
|||
| To handle audio/video sessions, what we usually known as *calls*, the network | |||
| requires to identify both involved parties: caller, the origin, and the callee, | |||
| the destination. Once identified the endpoints required to be connected, then | |||
| the destination. Once identified the involved communication endpoints, then | |||
| it is required to define *how* to send media from one endpoint to the other. | |||
There was a problem hiding this comment.
| it is required to define *how* to send media from one endpoint to the other. | |
| it is required to define *how* to send media from one endpoint to the other. |
| it is required to define *how* to send media from one endpoint to the other. | |
| the endpoints need to negotiate *how* to send media from one endpoint to the other. |
| - $ref: '#/components/parameters/hdrTransactionId' | ||
| - $ref: '#/components/parameters/hdrClientId' | ||
| - $ref: "#/components/parameters/x-correlator" | ||
| - $ref: '#/components/parameters/hdrRegistrationId' |
There was a problem hiding this comment.
Do you still need this header parameter and its definition (line 392) ?
| Typical example of session is a "single one-to-one call". | ||
| - **mediaSessionId**: Unique identifier of a Voice or Video **session** using | ||
| webrtc-call-handling API. Retrieved using the `webrtc-call-handling` API | ||
| - **x-correlator**: Unique ID associated to the REST _transaction_ created by the |
There was a problem hiding this comment.
see proposed update of the definition in the webrtc-call-handling yaml
| servers: | ||
| - url: '{apiRoot}/webrtc-registration/v0.2rc1' | ||
| - url: '{apiRoot}/webrtc-registration/v0.2' | ||
| description: APIs to manage Client Registration and Connection |
There was a problem hiding this comment.
| description: APIs to manage Client Registration and Connection | |
| description: APIs to manage device registration and connection |
| - name: Registration and Connection Management | ||
| description: APIs for Client to Register into MNO's IMS Network | ||
| - name: Registration | ||
| description: Operations to manage client registration into the network |
There was a problem hiding this comment.
| description: Operations to manage client registration into the network | |
| description: Operations to manage device registration with the network |
| summary: Create a registration | ||
| description: |- | ||
| API endpoint used to create a new registration of a device in the network. | ||
| It uses as main body data the `deviceId` to store its reference. |
There was a problem hiding this comment.
| It uses as main body data the `deviceId` to store its reference. | |
| It uses the `deviceId` in the request body. |
What type of PR is this?
Add one of the following kinds:
What this PR does / why we need it:
Latests comments from M3 review and M4 review preparations
Which issue(s) this PR fixes:
Fixes #70
Special notes for reviewers:
Changelog input
Additional documentation