Skip to content

Conversation

@XunliYang
Copy link
Contributor

@XunliYang XunliYang commented Jun 10, 2025

What type of PR is this?

Add one of the following kinds:

  • subproject management

What this PR does / why we need it:

Prepare the release r1.1 with v0.1.0-rc.1 API version:

  • Update CHANGELOG.md
  • Update API Readiness Checklist
  • Update versions in all files

Which issue(s) this PR fixes:

Fixes #58

Changelog input

 release-note

Additional documentation

This section can be blank.

docs

@chinaunicomyangfan chinaunicomyangfan added repository management Indicating issues with repository or release management Fall25 Issue in scope of Fall 25 (or under discussion for it) labels Jun 10, 2025
@hdamker
Copy link
Contributor

hdamker commented Jun 10, 2025

Created camaraproject/ReleaseManagement#218 for the review by Release Management team.

Note: as this is a release candidate for Fall25 it needs to be aligned with the upcoming releases r3.2 of Commonalities and ICM.

@XunliYang XunliYang closed this Jun 11, 2025
@XunliYang XunliYang reopened this Jun 11, 2025
@XunliYang
Copy link
Contributor Author

Created camaraproject/ReleaseManagement#218 for the review by Release Management team.

Note: as this is a release candidate for Fall25 it needs to be aligned with the upcoming releases r3.2 of Commonalities and ICM.

Thanks herbert, I have modified based on the release r3.2 of Commonalities and ICM. However there is not release r3.2 yet, I can't link to it. So I linked to r3.1 for the moment.

Copy link
Collaborator

@tanjadegroot tanjadegroot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Starts looking great ! please apply proposed changes.

| 2 | Design guidelines from Commonalities applied | O | M | M | M | Y | |
| 3 | Guidelines from ICM applied | O | M | M | M | Y | |
| 4 | API versioning convention applied | M | M | M | M | Y | v0.1.0 |
| 2 | Design guidelines from Commonalities applied | O | M | M | M | Y | [r3.1](https://github.com/camaraproject/Commonalities/releases/tag/r3.1) |
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
| 2 | Design guidelines from Commonalities applied | O | M | M | M | Y | [r3.1](https://github.com/camaraproject/Commonalities/releases/tag/r3.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 | Y | |
| 4 | API versioning convention applied | M | M | M | M | Y | v0.1.0 |
| 2 | Design guidelines from Commonalities applied | O | M | M | M | Y | [r3.1](https://github.com/camaraproject/Commonalities/releases/tag/r3.1) |
| 3 | Guidelines from ICM applied | O | M | M | M | Y | [r3.1](https://github.com/camaraproject/IdentityAndConsentManagement/releases/tag/r3.1) |
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
| 3 | Guidelines from ICM applied | O | M | M | M | Y | [r3.1](https://github.com/camaraproject/IdentityAndConsentManagement/releases/tag/r3.1) |
| 3 | Guidelines from ICM applied | O | M | M | M | Y | [r3.2](https://github.com/camaraproject/IdentityAndConsentManagement/releases/tag/r3.2) |

XunliYang and others added 12 commits June 17, 2025 10:31
Co-authored-by: Tanja de Groot <87864067+tanjadegroot@users.noreply.github.com>
Co-authored-by: Tanja de Groot <87864067+tanjadegroot@users.noreply.github.com>
Co-authored-by: Tanja de Groot <87864067+tanjadegroot@users.noreply.github.com>
Co-authored-by: Tanja de Groot <87864067+tanjadegroot@users.noreply.github.com>
Co-authored-by: Tanja de Groot <87864067+tanjadegroot@users.noreply.github.com>
@XunliYang
Copy link
Contributor Author

@tanjadegroot Thank you for such detailed suggestions. I've finished the revisions based on your recent comments. Please take a moment to review them.

@XunliYang
Copy link
Contributor Author

@hdamker @tanjadegroot : Thank you for the detailed review, we have now fixed all mandatory changes.
Please kindly re-review and provide comments or approval. Much appreciated!
Here is the update for the remain comment received:

  • Rename the API definition file as network-slice-booking.yaml
    fixed
  • Define the XCorrelator schema
    fixed
  • Use the XCorrelator schema within feature files with : And the header "x-correlator" complies with the schema at "#/components/schemas/XCorrelator"
    fixed
  • API Definition: Uses network-slice-booking:sessions:read change to network-slice-booking:sessions:get
    fixed
  • Schema TimePeriod fix : Add RFC 3339 description
    fixed
  • GET operation changes from "NSB Sessions" to "Network Slice Booking Sessions" align with other operation
    fixed
  • Test File Typos
    fixed

tanjadegroot
tanjadegroot previously approved these changes Jul 15, 2025
Copy link
Collaborator

@tanjadegroot tanjadegroot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would recommend making the changes on the property names to lowerCamelCase as soon as possible to avoid redoing API implementations, and the description fields for M4.

Once the lowerCamelCase updates are done (I do not put them as blocking the merge)
/LGTM from Release Management - great work by the team !

post:
tags:
- Network Slice Booking Sessions
summary: Creates a new session
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The description field is missing. This should normally contain the API endpoint documentation to help the developers understand the specificities of the endpoint API.

tags:
- Network Slice Booking Sessions
summary: Delete a NSB session
description: Deleting NSB session
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally this should have some explanation about delete usage e.g. what is the effect of deleting the session befor the endtime that is was booked for, etc.

tags:
- Network Slice Booking Sessions
summary: Get NSB session information
description: Querying for NSB session resource information details
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The description may contain more details about the provided resource information that is returned when calling this end-point.

NOTE. you can improve the descriptions also later for M4

description: Attributes required to create a session
type: object
properties:
ServiceTime:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

property and similar names follow lowerCamelCase naming pattern: serviceTime, serviceArea, qosProfile, maxNumOfDevices, downStreamRatePerDevice, etc.

Schema names follow UpperCamelCase naming pattern so ideally the "QoSProfile" should be "QosProfile" (with a lower case "s").

these rules help in readability.

These updates could also wait for M4 but would have impact on test API implementations.

These updates will also impact the .feature files, and possibly other e.g. the documentation file(s)

type: object
properties:
value:
description: Quantity of duration
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
description: Quantity of duration
description: Length of duration

@XunliYang
Copy link
Contributor Author

XunliYang commented Jul 16, 2025

I would recommend making the changes on the property names to lowerCamelCase as soon as possible to avoid redoing API implementations, and the description fields for M4.

Once the lowerCamelCase updates are done (I do not put them as blocking the merge) /LGTM from Release Management - great work by the team !

@tanjadegroot : Thanks Tanja,we have finished the lowerCamelCase changes per your suggestion. And for the description fileds, we created the issue #64 to track it and will fix it in M4 with more detailed description:

Requesting re-review on the updates - kindly provide your approval or comments when available. Many thanks!

* **serviceTime**: is defined by a start time and an end time, which indicates the period during which the network slice will be reserved.
* **serviceArea**: can be defined as a circle or a polygon, allowing for flexible geographical coverage.
* **qosProfile**: includes parameters such as maximum number of devices, downstream and upstream rate per device, and packet delay budget.
* **SessionId**: is a unique identifier for the session, which is returned when a session is created and used to retrieve or delete the session later.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, many thanks for doing these updates. I did not list exhaustively all the names to be updated.

please check for other cases as well, e.g.
SessionId --> sessionId

thanks !

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apologies for the oversight. We've double-checked and updated all cases. I think I have corrected all in commit. Your feedback is greatly appreciated! Let us know if you spot anything else!

- serviceArea
- qosProfile

QosProfile:
Copy link
Contributor

@hdamker hdamker Jul 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have already a definition for QosProfile within the API qos-profiles in QualityOnDemand which is different from the definition here (as defined as profile for one device).

But as the same schema is already used in multiple APIs, I would prefer to use another name for the profile which is specific for NetworkSliceBooking, e.g. NetworkSliceQosProfile, to avoid confusions.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Herbert, this indeed makes sense. We have created issue #65 to track it. How about SliceQosProfile? Maybe it‘s more brief and also indicates the qosProfile for sclice.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SliceQosProfile is also fine for me.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hdamker @tanjadegroot
Thanks for your comments, we have corrected naming pattern in Naming pattern correction, and modified the QosProfile to SliceQosProfile in Modify the name for schema QosProfile to avoid confusions.
Could you please review the updates again and share your approval or feedback at your earliest convenience? Thank you!

@XunliYang XunliYang requested a review from hdamker July 25, 2025 09:18
Copy link
Contributor

@hdamker hdamker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@XunliYang As far as I can see are all review comments from @tanjadegroot addressed which is great. Unfortunately I found with a little help of AI some further issues.

High priority (should be addressed before release)

Replace 204 usage with appropriate error codes (400/422)

  • Issue: HTTP 204 means success and "No Content" and should not have a response body. Using it for failures is non-standard.
  • Fix: Use appropriate 4xx codes (e.g., 422 UNPROCESSABLE_ENTITY or 400 with specific error codes). Correct also the test definition, the tests expect response body for 204, which contradicts HTTP standards.

Standardize error message enums

  • Issue: Inconsistent Error Message Format
message:
  enum:
    - SERVICEAREA NOT SUPPORTED  # Has space
    - RESOURCES INSUFFICIENT

Fix: Use consistent format and consider to define as API specific: NETWORK_SLICE_BOOKING.SERVICEAREA_NOT_SUPPORTED
NETWORK_SLICE_BOOKING.RESOURCES_INSUFFICIENT

Note: the definition as specific API error codes I see as medium priority, if you address it, please check for other API specific error codes, and cf. CAMARA-API-Design-Guide.md#31-standardized-use-of-camara-error-responses

Low priority (to be considered after M3)

Add description to operation

  • Missing description for POST /sessions

@XunliYang
Copy link
Contributor Author

@hdamker : Thanks for pointing this out - great catch! We'd actually considered standardizing these operation error codes and creating custom business logic failure codes before realizing it wasn't the right approach. Really appreciate you flagging this. Here are the changes we've implemented:

error codes

We have modified in commit Modify error codes usage with appropriate error codes. The changes include:

  • POST operation changed error code 204 to 422, and error message consolidated into a single API-specific definition:NETWORK_SLICE_BOOKING.RESOURCES_NOT_APPLICABLE
  • POST operation changed error code 200 to 201
  • Delete operation changed error code 200 to 204

operation description

We have modified in Add operation description. Further updates will be continuously provided and tracked at #64 .

Copy link
Collaborator

@tanjadegroot tanjadegroot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few more changes and your good to go !

"404":
$ref: "#/components/responses/Generic404"
"429":
$ref: "#/components/responses/Generic429_requests"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
$ref: "#/components/responses/Generic429_requests"
$ref: "#/components/responses/Generic429"

"404":
$ref: "#/components/responses/Generic404"
"429":
$ref: "#/components/responses/Generic429_requests"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
$ref: "#/components/responses/Generic429_requests"
$ref: "#/components/responses/Generic429"

code: TOO_MANY_REQUESTS
message: Either out of resource quota or reaching rate limiting.

Generic429_requests:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this schema can be removed as it is the same as the Generic429. I have proposed above the update of the 2 occurrences of this one to be Generic429 instead.

Also align the examples section of the Generi429 error definition with the definitions in CAMARA_common.yaml (see https://github.com/camaraproject/Commonalities/blob/r3.2/artifacts/CAMARA_common.yaml)

Maybe also check the other error examples to be aligned

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Tanja. We treat Generic429_request as a separate case from Generic429 since POST operation contains the extra error code QUOTA_EXCEEDED. Thus, maybe can we just modify the code message to align with the definitions in CAMARA_common.yaml and keep the Generic429_request?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @XunliYang If you compare with the latest in https://github.com/camaraproject/Commonalities/blob/r3.2/artifacts/CAMARA_common.yaml the Generic429 seems the same as your definition of Generic429_requests - so ideally you would use the name from CAMARA_common one but remove the code the QUOTA_EXCEEDED one.

I will verify if that is allowed, but the Guidelines do not say that all error codes need to be supported by the API provider.

For now, let's keep what you have done to meet the M3 deadline

@@ -1,5 +1,5 @@
@NetworkSliceBooking
Feature: CAMARA Network Slice Booking API v0.1.0 - Operations for createSession
Feature: CAMARA Network Slice Booking API v0.1.0-rc.1 - Operations for createSession
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Feature: CAMARA Network Slice Booking API v0.1.0-rc.1 - Operations for createSession
Feature: CAMARA Network Slice Booking API v0.1.0-rc.1 - Operation createSession

Given starttime, endtime, the request body property "$.ServiceArea" is set to the service area of not supported and the configuration of information of network slicing
Given starttime, endtime, the request body property "$.serviceArea" is set to the service area of not supported and the configuration of information of network slicing
When the request "createSession" is sent
Then the response status code is 204
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need to update this to the same error code as done in the API yaml, instead of a success code (204).

same for the other 204 "fail" scenarios below.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

line 132 and 143: update the messages to the same as in the Generic429 definition (per CAMARA_common.yaml)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apply same changes as in comments on the createSession.feature file (if applicable)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apply same changes as in comments on the createSession.feature file (if applicable)

@XunliYang
Copy link
Contributor Author

@tanjadegroot: Thanks for your valuable feedback. We've implemented the following modifications accordingly in commit Modify error codes 200 201 429:

  • Generic429 and Generic429_request
    Remained Generic429_request, and modified the code message both to align with the definitions in CAMARA_common.yaml.(To be determinded)
  • Operations for createSession to Operation createSession
    Fixed in all feature files.
  • Error code 200 and 201
    Streamlined the response with removing status and result(200 response duplicates semantics #67 ). And added SessionInfo to the response of 201.
  • Feature files
    The modifications related to error codes have been synchronized across all feature files.

Copy link
Collaborator

@tanjadegroot tanjadegroot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the hard work on this API.
It looks great !
/LGTM from Release Management

code: TOO_MANY_REQUESTS
message: Either out of resource quota or reaching rate limiting.

Generic429_requests:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @XunliYang If you compare with the latest in https://github.com/camaraproject/Commonalities/blob/r3.2/artifacts/CAMARA_common.yaml the Generic429 seems the same as your definition of Generic429_requests - so ideally you would use the name from CAMARA_common one but remove the code the QUOTA_EXCEEDED one.

I will verify if that is allowed, but the Guidelines do not say that all error codes need to be supported by the API provider.

For now, let's keep what you have done to meet the M3 deadline

@XunliYang XunliYang requested a review from hdamker July 31, 2025 02:28
@XunliYang
Copy link
Contributor Author

@hdamker: We believe all requested changes have been addressed. Could you kindly re-review and share your feedback or approval? Thank you for your time!

@hdamker hdamker dismissed their stale review July 31, 2025 09:30

I trust that the changes are done, but don't have the time for a review.

@tanjadegroot
Copy link
Collaborator

@XunliYang the review is complete.

I checked on the error code, and indeed you are allowed to remove error code from the list for the Generic429 to adust it to the API need (as for other optional error codes (see Commonalities API Guide section 3.1 under the tables):

Error statuses 400, 404, 409, 422, ****: These error statuses SHOULD be documented based on the API design and the functionality involved. Subprojects evaluate the relevance and necessity of including these statuses in API specifications.

so I would still recommend to align the error name to Generic429 instead of an API specific one before merging if you have time, or create an issie in the repo to address it later.

@XunliYang
Copy link
Contributor Author

@XunliYang the review is complete.

I checked on the error code, and indeed you are allowed to remove error code from the list for the Generic429 to adust it to the API need (as for other optional error codes (see Commonalities API Guide section 3.1 under the tables):

Error statuses 400, 404, 409, 422, ****: These error statuses SHOULD be documented based on the API design and the functionality involved. Subprojects evaluate the relevance and necessity of including these statuses in API specifications.

so I would still recommend to align the error name to Generic429 instead of an API specific one before merging if you have time, or create an issie in the repo to address it later.

@tanjadegroot @hdamker Thanks for your suggestions. We will merge this PR to meet the M3 deadline. Also we have created an issue #68 to track it later and modify after this PR. However I see that other APIs also have specific error names such as QOD's CreateSessionBadRequest400 and GenericExtendSessionDuration400. We have both error names Generic429 and Generic429_request. Thus I'm wondering if I need to only use Generic429 in different operations.

@chinaunicomyangfan chinaunicomyangfan merged commit 3542382 into camaraproject:main Aug 1, 2025
1 check passed
@tanjadegroot
Copy link
Collaborator

tanjadegroot commented Aug 1, 2025

@tanjadegroot @hdamker Thanks for your suggestions. We will merge this PR to meet the M3 deadline. Also we have created an issue #68 to track it later and modify after this PR. However I see that other APIs also have specific error names such as QOD's CreateSessionBadRequest400 and GenericExtendSessionDuration400. We have both error names Generic429 and Generic429_request. Thus I'm wondering if I need to only use Generic429 in different operations.

yes, you can make your own error codes as needed, but then they are not "GenericXxx", but it has a specific name for the specific error. In your case, the error is the same as Generic429 with one less option in the codes enum, so no need for a specific error definition.

The QoD API was one of the first APIs defined in CAMARA, and was done before the Commonalities rules for error codes were established. So they may have some deviations for backward compatibility reasons.

For new APIs, we try to make developers life as easy as possible by having consistent usage in specifications.

Thank you very much for considering this update.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Fall25 Issue in scope of Fall 25 (or under discussion for it) repository management Indicating issues with repository or release management

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Release candidate for Fall25 M3 milestone

5 participants