Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Split QoD in Quality-On-Demand and QoS Profiles APIs #289

Merged
merged 8 commits into from
May 17, 2024

Conversation

jlurien
Copy link
Collaborator

@jlurien jlurien commented Apr 30, 2024

What type of PR is this?

  • enhancement/feature

What this PR does / why we need it:

QoD v0.10.x API is split in 2:

  • Quality-On-Demand, version WIP, with /sessions operations

    • PR suggests "Quality-On-Demand" and /quality-on-demand, as API Title and basePath
  • QoS Profiles, version WIP, with /qos-profiles operations

    • PR suggests "QoS Profiles" and /qos-profiles, as API Title and basePath

The discussion about the names is not closed, so they may change.

Considerations:

  • yaml files in /code folder. We may consider a different organization, e.g. a subfolder per API.
  • All schemas moved to the spec where they are referred. Only the schema QosProfileName is used in both.
  • securitySchemas is only kept in Quality-On-Demand. TBD which securityScheme to be applied to QoS Profiles, if any.
  • Old info.description is kept mostly within Quality-On-Demand. A basic one is included for QoS Profiles, but may be enhanced. A cross reference between both APIs could be added.
  • versions are "wip", and version in basePaths are /v-wip

Which issue(s) this PR fixes:

Fixes #265
Discussion is not closed. It was decided in last meeting that "we will discuss final names based on the PR."

Special notes for reviewers:

README and other files in documentation will likely need to be adapted

Changelog input

- QoD API is split in 2: one for operations on sessions, and other for operations on qos-profiles

@jlurien
Copy link
Collaborator Author

jlurien commented Apr 30, 2024

This error requires some fixing in the CI, as now is dependent on the spec filename:

Run `npm audit` for details.
Run cd /home/runner/work/_actions/char0n/swagger-editor-validate/v1.3.2 && SWAGGER_EDITOR_URL=https://editor.swagger.io/ DEFINITION_FILE=code/API_definitions/qod-api.yaml IGNORE_ERROR= node src/index.js
Error: Error while validating in Swagger Editor
Error: Error: ENOENT: no such file or directory, open '/home/runner/work/QualityOnDemand/QualityOnDemand/code/API_definitions/qod-api.yaml'
Error: Process completed with exit code 1.

@hdamker
Copy link
Collaborator

hdamker commented May 3, 2024

From QoD meeting on May 3rd:

  • Naming will be accepted as proposed for this PR ... can be discussed separately and changed if needed
  • One week of review ... will be merged on next Friday (May 10th) if no open comments.

@hdamker
Copy link
Collaborator

hdamker commented May 3, 2024

@camaraproject/quality-on-demand_maintainers - I just learned that a team will be removed from the reviewer list as soon as one member of the team has reviewed "on behalf" (here @RandyLevensalor). But I suppose that all team members have at least got the review request ones?

Anyway, this PR is important to get reviewed and merged, so that we continue with other issues. Please be aware of the deadline of March 10th for your reviews.

@RandyLevensalor
Copy link
Collaborator

This error requires some fixing in the CI, as now is dependent on the spec filename:

Run `npm audit` for details.
Run cd /home/runner/work/_actions/char0n/swagger-editor-validate/v1.3.2 && SWAGGER_EDITOR_URL=https://editor.swagger.io/ DEFINITION_FILE=code/API_definitions/qod-api.yaml IGNORE_ERROR= node src/index.js
Error: Error while validating in Swagger Editor
Error: Error: ENOENT: no such file or directory, open '/home/runner/work/QualityOnDemand/QualityOnDemand/code/API_definitions/qod-api.yaml'
Error: Process completed with exit code 1.

This fix has been merged. When the branch is rebased it should resolve this issue. @hdamker thanks for the quick fix.

Copy link
Collaborator

@RandyLevensalor RandyLevensalor left a comment

Choose a reason for hiding this comment

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

Can the README.md also be updated to reflect these changes?

The links to the redoc and swagger views of these files will be missing along with some of the other descriptions.

code/API_definitions/qos-profiles.yaml Outdated Show resolved Hide resolved
code/API_definitions/qos-profiles.yaml Show resolved Hide resolved
@hdamker
Copy link
Collaborator

hdamker commented May 13, 2024

Can the README.md also be updated to reflect these changes?

The links to the redoc and swagger views of these files will be missing along with some of the other descriptions.

@RandyLevensalor that will be done when we create a (public) release. Currently they are pointing correctly to the latest released version.

@hdamker hdamker requested a review from a team May 17, 2024 08:38
@hdamker
Copy link
Collaborator

hdamker commented May 17, 2024

Hi, as we have todo the QoD Call I kindly ask @camaraproject/quality-on-demand_maintainers for their review. Hopefully we can discuss today the outstanding comments and get the PR done.

Copy link
Collaborator

@sfnuser sfnuser left a comment

Choose a reason for hiding this comment

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

Thanks.

jlurien and others added 3 commits May 17, 2024 12:14
Co-authored-by: Herbert Damker <52109189+hdamker@users.noreply.github.com>
Co-authored-by: Herbert Damker <52109189+hdamker@users.noreply.github.com>
Co-authored-by: Eric Murray <eric.murray@vodafone.com>
@jlurien
Copy link
Collaborator Author

jlurien commented May 17, 2024

@hdamker @eric-murray @RandyLevensalor Suggestions merged

@hdamker hdamker merged commit 8ac23e1 into camaraproject:main May 17, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Proposal to split QoS Sessions and QoS Profiles into two separate API definitions
5 participants