QoS B: Alignment with Commonalities r4.3#97
Conversation
#563, 'Addr' schema objects updated to 'Address'
jlurien
left a comment
jlurien
left a comment
There was a problem hiding this comment.
Thanks @Masa8106, I amde some suggestions.
We probably should consider replacing schemas available in CAMARA_common.yaml with $ref.
Directly replaceable — equivalent after this PR's changes
| Schema | Notes |
|---|---|
ErrorInfo |
PR adds format: int32, minimum/maximum on status, maxLength on code/message — same values as common |
XCorrelator |
PR adds description and maxLength: 256 — matches common |
Device |
Structurally equivalent; common has an updated NOTE2 referencing the current Commonalities release |
DeviceResponse |
Equivalent structure |
DeviceIpv4Address |
PR renames from DeviceIpv4Addr and adds the same constraints as common |
SingleIpv4Address |
PR renames from SingleIpv4Addr and adds maxLength: 15 — matches common |
DeviceIpv6Address |
PR adds maxLength: 45 — matches common |
PointList, Point, Latitude, Longitude |
Identical |
x-correlator parameter and header |
Equivalent |
Generic400, Generic401, Generic404, Generic410, Generic422, Generic429 |
Equivalent |
PR introduces values that contradict CAMARA_common.yaml
| Schema | This PR | CAMARA_common.yaml |
Issue |
|---|---|---|---|
PhoneNumber.maxLength |
20 |
16 |
E.164 max is + + 15 digits = 16 chars; 20 is incorrect |
NetworkAccessIdentifier.maxLength |
256 |
2048 |
NAI format allows up to 2048 chars; 256 is too restrictive |
I have make the suggestions as comments, but replacing these with $ref to the common file would apply the correct values automatically.
We should merge #70 prior to this one and sync this with master.
| minItems: 0 | ||
| maxItems: 100 | ||
|
|
||
| SinkCredential: |
There was a problem hiding this comment.
Any reason not to replace this with $ref: "../common/CAMARA_event_common.yaml#/components/schemas/SinkCredential"?
There was a problem hiding this comment.
When this PR was opened, target release was alpha release and it was not warning not to use $ref;. This is a reason.
Now I know it is warning not to use $ref: for RC release. So let me update it to use $ref: after merging this PR.
Co-authored-by: Jose Luis Urien <jlurien@gmail.com>
Co-authored-by: Jose Luis Urien <jlurien@gmail.com>
Co-authored-by: Jose Luis Urien <jlurien@gmail.com>
|
Hi @jlurien, thank you for raising some suggestions. I am ok with all suggestions from you. |
|
Hi @jlurien, thank you for merging other PR about test scenario. |
|
For the feature file, it is better to solve it here before merging. For the task of alignment and $ref to CAMARA_common.yaml, I can do it in a separate PR after this one is merged. |
|
To solve the conflicet requires to use the command line (we cannot use Web UI). So let me give some time to solve it. |
- qos-booking.yaml: auto-merged cleanly (Commonalities r4.3 constraints from PR camaraproject#97 combined with CloudEvent alignment from PRs camaraproject#98 and camaraproject#102) - qos-booking.feature (monolithic): deleted, superseded by split files introduced in PR camaraproject#70; $.device assertion updated in qos-booking-getBookingById.feature and qos-booking-retrieveBookingByDevice.feature to match PR camaraproject#97 intent
|
@Masa8106, I have resolved the conflict myself. Please review everything is fine with the latest changes and merge if it is fine for you |
|
Hi @jlurien, much appreciated that you have kindly resoleved conflicts. Please let me do final review before merge. |
2 participants
What type of PR is this?
What this PR does / why we need it:
Which issue(s) this PR fixes:
Fixes #84
Special notes for reviewers:
Changelog input