Conversation
CHANGELOG.md
Outdated
|
|
||
| ## Table of Contents | ||
|
|
||
| - [v0.1.0 - Initial contribution](#v010---initial-contribution) (Fall25 public release) |
There was a problem hiding this comment.
Please use r1.1 as headline and below, see https://github.com/camaraproject/ReleaseManagement/blob/main/documentation/CHANGELOG_TEMPLATE.md and example in e.g. https://github.com/camaraproject/CallForwardingSignal/blob/main/CHANGELOG.md
There was a problem hiding this comment.
thanks @hdamker - draft PR updated to align with Changelog guidelines
|
The release PR needs to update all files containing the API version, not only the changelog: |
|
thanks @tanjadegroot - I was waiting for the last PR to be merged (which happened this morning). I have updated this PR now to include everything. Will you take another look when you have a minute? I think I have everything for Fall 25 now in this PR. Thanks! |
|
Hi Ben, thanks ! |
|
Please see the automated review results here: some issues need to be fixed. |
| @@ -1,7 +1,7 @@ | |||
| openapi: 3.0.3 | |||
| info: | |||
| title: Session Insights API | |||
There was a problem hiding this comment.
| title: Session Insights API | |
| title: Session Insights |
|
HI @benhepworth - I would normally be happy to contribute the required changes to the PR as a codeowner, but as the PR comes from a branch outside CAMARA I don't have the write access to do so. Best I can do is the suggestion above which fixes one critical error (only certain code blocks within the proximity of the change can be 'suggested' in this way). BTW you'll find the text to fix the critical 'Authorization Template' issue here: |
tanjadegroot
left a comment
There was a problem hiding this comment.
Review of the API-Readiness-Checklist only.
Needs updates
| | 2 | Design guidelines from Commonalities applied | O | M | M | M | Y | Comm. release nr | | ||
| | 3 | Guidelines from ICM applied | O | M | M | M | tbd | ICM release nr | | ||
| | 4 | API versioning convention applied | M | M | M | M | tbd | | | ||
| | 2 | Design guidelines from Commonalities applied | O | M | M | M | Y | [r1.1](https://github.com/camaraproject/SessionInsights/releases/tag/r1.1) | |
There was a problem hiding this comment.
| | 2 | Design guidelines from Commonalities applied | O | M | M | M | Y | [r1.1](https://github.com/camaraproject/SessionInsights/releases/tag/r1.1) | | |
| | 2 | Design guidelines from Commonalities applied | O | M | M | M | Y | [r3.2](https://github.com/camaraproject/Commonalities/releases/tag/r3.2) | |
| | 3 | Guidelines from ICM applied | O | M | M | M | tbd | ICM release nr | | ||
| | 4 | API versioning convention applied | M | M | M | M | tbd | | | ||
| | 2 | Design guidelines from Commonalities applied | O | M | M | M | Y | [r1.1](https://github.com/camaraproject/SessionInsights/releases/tag/r1.1) | | ||
| | 3 | Guidelines from ICM applied | O | M | M | M | Y | [r1.1](https://github.com/camaraproject/SessionInsights/releases/tag/r1.1) | |
There was a problem hiding this comment.
| | 3 | Guidelines from ICM applied | O | M | M | M | Y | [r1.1](https://github.com/camaraproject/SessionInsights/releases/tag/r1.1) | | |
| | 3 | Guidelines from ICM applied | O | M | M | M | Y | [r3.2](https://github.com/camaraproject/IdentityAndConsentManagement/releases/tag/r3.2) | |
| | 4 | API versioning convention applied | M | M | M | M | tbd | | | ||
| | 2 | Design guidelines from Commonalities applied | O | M | M | M | Y | [r1.1](https://github.com/camaraproject/SessionInsights/releases/tag/r1.1) | | ||
| | 3 | Guidelines from ICM applied | O | M | M | M | Y | [r1.1](https://github.com/camaraproject/SessionInsights/releases/tag/r1.1) | | ||
| | 4 | API versioning convention applied | M | M | M | M | Y | | |
There was a problem hiding this comment.
| | 4 | API versioning convention applied | M | M | M | M | Y | | | |
| | 4 | API versioning convention applied | M | M | M | M | Y | 0.1.0-rc.1 | |
| | 10 | API release numbering convention applied | M | M | M | M | Y | | | ||
| | 11 | Change log updated | M | M | M | M | Y | [/CHANGELOG.md](/CHANGELOG.md) | | ||
| | 12 | Previous public release was certified | O | O | O | M | N | | | ||
| | 13 | API description (for marketing) | O | O | M | M | Y | [wiki link](https://lf-camaraproject.atlassian.net/wiki/spaces/CAM/pages/94011619/SessionInsights) | |
There was a problem hiding this comment.
| | 13 | API description (for marketing) | O | O | M | M | Y | [wiki link](https://lf-camaraproject.atlassian.net/wiki/spaces/CAM/pages/94011619/SessionInsights) | | |
| | 13 | API description (for marketing) | O | O | M | M | Y | [wiki link](https://lf-camaraproject.atlassian.net/wiki/spaces/CAM/pages/90538343/ConnectivityInsights+Session+Insights+API+description) | |
| **Initial contribution of Session Insights API definition, including initial documentation, linting, test cases, and OpenAPI spec.** | ||
|
|
||
| - API definition **with inline documentation**: | ||
| - OpenAPI [YAML spec file](https://github.com/camaraproject/SessionInsights/blob/r1.1/main/code/API_definitions/session-insights.yaml) |
There was a problem hiding this comment.
| - OpenAPI [YAML spec file](https://github.com/camaraproject/SessionInsights/blob/r1.1/main/code/API_definitions/session-insights.yaml) | |
| - OpenAPI [YAML spec file](https://github.com/camaraproject/SessionInsights/blob/r1.1/code/API_definitions/session-insights.yaml) |
| - @benhepworth did most of the work on this initial release | ||
| - @kevsy helped out and co-authored several PRs and participated in PR reviews | ||
| - This "release" is only tagged to document the history of the API, it is not intended to be used by implementors or API customers | ||
| - It was originally an implementation by the CableLabs team, and is now maintained by the Camara Project |
There was a problem hiding this comment.
| - It was originally an implementation by the CableLabs team, and is now maintained by the Camara Project | |
| - It was originally an implementation by the CableLabs team, and is now maintained by the CAMARA Project |
There was a problem hiding this comment.
line 10: the concept of API "family" is deprecated --> In this paragraph, change both "Connectivity Insights API family" and "Connectivity Insights API" to "Connectivity Insights API group".
line 64: Camara --> CAMARA
There was a problem hiding this comment.
lines 77 / 78: should the tags section not have "Session Insights" and the description related to that ?
There was a problem hiding this comment.
some comments to better understand the API:
In the info.description field:
-
instead of "network operator", use "API provider".
-
I am confused between:
- the parameter SessionId (schema defined line 312)
- the id property of the SessionBase object (line 354)
- the applicationSessionId also a property of the SessionBase object (line 358)
they have very little or no description and I also do not see it clarified elsewhere, maybe one is more intented as a name ? If we need all of these ids, what is the relation between them (if any), and what is their purpose
-
some info is missing on the cardinality of object relation:
- 1 device, multiple applications ?
- application, 1 application-profile ?
- 1 application, multiple sessions ?
- 1 session, ... ?
-
the Authorization and Authentication section: how is the link made between this section on device and the actual APIs ? extend the section with some info about this
-
same for the next error code section
-
the term KPIs and KPI metrics seem to be used interchangeably - preferably pick only one
-
change "existing Application Profile" to "selected Application Profile" (from the list of available profiles obtained with the GET application-profiles API) - can be more explicity described in the info.description section
-
line 24: change "Application Profile Integration" to "Use of Application Profiles API"
-
RCA abbreviation not to be used - use "event indicating the problem cause" or something
-
line 33: what is WMM ? no abbreviations
There was a problem hiding this comment.
This API uses implicit notifications and e.g. the sink and sink credentials definitions should be aligned to the Event Guide - https://github.com/camaraproject/Commonalities/blob/r3.2/documentation/CAMARA-API-Event-Subscription-and-Notification-Guide.md
for detailed help on alignment please seealso the wiki: Commonalities r3.2 alignment please check (see https://lf-camaraproject.atlassian.net/wiki/spaces/CAM/pages/132218881/Analysis+of+Commonalities+0.6.0-rc.1+changes, e.g. for subscription APIs:
- including add sink pattern and specific 400 - INVALID_SINK error
- Update types property of SubscriptionRequest to allow more than one event type per subscription (optional)
- Update types property of SubscriptionRequest to use SubscriptionEventType schema (enum of defined types)
- 3.2 subscription-started event (optional event)
- 3.3 subscription-updated event (optional event)
- 3.4 subscription-ends --> subscription-ended (event name change)
tanjadegroot
left a comment
There was a problem hiding this comment.
main issue: security sections are missing in the API yaml file
There was a problem hiding this comment.
The error definitions are missing the examples section in the content.application/json part
please see: https://github.com/camaraproject/Commonalities/blob/r3.2/artifacts/CAMARA_common.yaml
line 236 and after
There was a problem hiding this comment.
the security aspects are missing from the API - securitySchemes in
- paths
- components section
- on the sink (aligned to Commonalities)
tanjadegroot
left a comment
There was a problem hiding this comment.
The README.md file should be updated
- in the release section, un-comment the applicable line to indicate the pre-release and add its release link
- change "network operator" to "API provider"
- the "service" to "session"
Overall documentation (including description fields in yaml) can be more elaborate to help API users.
|
Hi Team, this means
Please provide your feedback here latest tomorrow noon CET as officially M3 is closed by EOB today. |
|
Thanks @tanjadegroot - given the significant refactoring required to achieve compliance with commonalities and ICM, it will not be feasible to achieve these for M3. So I will proceed to update the tracker to reflect the Spring 26 target. |
Many thanks, @kevin. |
|
Hi Team, any plans for the Spring26 participation ? |
|
will be releasing via the new release process soon...closing this PR |
What type of PR is this?
Add one of the following kinds:
What this PR does / why we need it:
M3 release - v0.1.0 of SessionInsights
Which issue(s) this PR fixes:
Fixes #54
Fixes #
Special notes for reviewers:
Changelog input
Additional documentation
This section can be blank.