Skip to content

Conversation

@RandyLevensalor
Copy link
Collaborator

What type of PR is this?

Add one of the following kinds:

  • enhancement/feature

What this PR does / why we need it:

Adds new query capabilites

Which issue(s) this PR fixes:

Fixes #147

Special notes for reviewers:

Changelog input

 release-note
Enhanced filtering capabilities in the /retrieve-qos-profiles endpoint:
Added support for filtering by all QoS profile attributes including rates, durations, priority, latency, and service classes
Clarified comparison logic for different types of attributes:
Rate attributes: returned profiles have values greater than or equal to requested
Duration attributes:
minDuration: profiles have values less than or equal to requested
maxDuration: profiles have values greater than or equal to requested
Priority: profiles have values less than or equal to requested (lower = higher priority)
Latency attributes (jitter, packetDelayBudget): profiles have values less than or equal to requested
packetErrorLossRate: profiles have values less than or equal to requested
l4sQueueType and serviceClass: profiles match exactly

Additional documentation

This section can be blank.

docs

@JoachimDahlgren
Copy link

One one side I appreciate that you are able to filter on any of the attributes in the profile. On the other side from the working group meeting today I got the impression that very few of these attributes are set on QoS Profiles today. If that is the case adding a filter will only result in getting nothing back in the response.

Do we have any indication of information always set in the profile and if so will it make sense to start with enabling filtering only on those?

Also what makes sense from an API invoker perspective. What would be most important to filter on?

@RandyLevensalor
Copy link
Collaborator Author

One one side I appreciate that you are able to filter on any of the attributes in the profile. On the other side from the working group meeting today I got the impression that very few of these attributes are set on QoS Profiles today. If that is the case adding a filter will only result in getting nothing back in the response.

Do we have any indication of information always set in the profile and if so will it make sense to start with enabling filtering only on those?

Also what makes sense from an API invoker perspective. What would be most important to filter on?

@JoachimDahlgren thanks for the feedback. I don't think that the API should limit what can be filtered. These decisions should be left to the API provider.

I think we provide a mechanism and documentations to clearly state how to handle when attributes are not set.

One option would be to return an error 422 query attribute not supported. We could also add a new get call to get a list of supported query attributes to better automate the process.

Operators could also provide a default value for attributes that are not customizable through the API.

@eric-murray
Copy link
Collaborator

One one side I appreciate that you are able to filter on any of the attributes in the profile. On the other side from the working group meeting today I got the impression that very few of these attributes are set on QoS Profiles today. If that is the case adding a filter will only result in getting nothing back in the response.

As I mentioned in the meeting last Friday, my preference is to allow the API consumer to include a flag in their request indicating whether or not they want profiles included in the response that do not have a value set for an attribute they are filtering on.

@RandyLevensalor
Copy link
Collaborator Author

One one side I appreciate that you are able to filter on any of the attributes in the profile. On the other side from the working group meeting today I got the impression that very few of these attributes are set on QoS Profiles today. If that is the case adding a filter will only result in getting nothing back in the response.

As I mentioned in the meeting last Friday, my preference is to allow the API consumer to include a flag in their request indicating whether or not they want profiles included in the response that do not have a value set for an attribute they are filtering on.

@eric-murray I like this proposal. Should this flag be required or optional and if optional, is there a suggestion for the default value?

I tend to lean towards a simpler API call, which would have this flag as optional and to default to only returning matching profiles. I'm open to feedback on this point.

@eric-murray
Copy link
Collaborator

I tend to lean towards a simpler API call, which would have this flag as optional and to default to only returning matching profiles. I'm open to feedback on this point.

I agree - having a default value implicitly makes it an optional parameter (for the API consumer), and I agree the default should be to narrow the range of responses to profiles for which values are specified for the filter parameters

@eric-murray
Copy link
Collaborator

An additional comment. For documentation, I'd prefer if the applicability of a given filter parameter (i.e. is it and exact match, or greater or equal to, or less than or equal to) be specified in a description property for each individual parameter, rather than being in the overall description for the request body. This makes it easier for the API consumer to understand how the filter will be applied when they are looking at the parameter itself.

This can be done using the allOf trick we use elsewhere. e.g. something like:

    QosProfileDeviceRequest:
      description: <short summary>
      type: object
      properties:
        targetMinUpstreamRate:
          description: returned profiles will have a target minimum upstream data rate greater than or equal to the requested value
          allOf:
            -  $ref: "#/components/schemas/Rate"

Copy link
Collaborator

@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.

I have to admit that I'm not in favor to add this functionality at all at the current time -- it will make the implementation of the API more complex, hence slowing the adoption across operators, and has from my perspective currently limited value for API consumer.

We have no experience yet how profiles will actually look like and how many will be there. We have omitted pagination, as the general assumption is that it will be a small number.

If we add the functionality there is at least one change needed.

Comment on lines +519 to +521
required:
- name
- status
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why are name and status required in this request parameter object? I don't think that this will return the expected results ...

QosProfileDeviceRequest:
description: |
Request object for QoS Profiles for a given device
Request object for QoS Profiles for a given device.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Looking on the description of the operation this sentence is not correct ... the operation allows also to use the request object without a given device.

BTW: the operation description might need to be updated as well a bit.

@hdamker
Copy link
Collaborator

hdamker commented Jun 20, 2025

As decided within Sub Project call on June 13th: Changing the PR to draft and backlog

@hdamker hdamker marked this pull request as draft June 20, 2025 17:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Extend the query capabilities for Qos Profiles

4 participants