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

Remove basePath in servers.url as variable #199

Closed
jlurien opened this issue Oct 6, 2023 · 3 comments · Fixed by #214
Closed

Remove basePath in servers.url as variable #199

jlurien opened this issue Oct 6, 2023 · 3 comments · Fixed by #214
Labels
enhancement New feature or request release management

Comments

@jlurien
Copy link
Contributor

jlurien commented Oct 6, 2023

Problem description
I noticed that we are misusing variables in servers.url properties in most API definitions.

Commonly we define servers as, e.g.:

servers:
  - url: "{apiRoot}/{basePath}"
    variables:
      apiRoot:
        default: http://localhost:9091
        description: API root
      basePath:
        default: qod/v0
        description: Base path for the QoD API

But basePath which is the api-name and major version is not something that may be changed by the client. default is defined in OAS as:

Field Name Type Description
default string REQUIRED. The default value to use for substitution, and to send, if an alternate value is not supplied. Unlike the Schema Object's default, this value MUST be provided by the consumer.

We are not allowing an alternate value to be supplied by the API client, so this part of the servers.url must be fixed.

Possible evolution
Proposal is to redefine servers as, e.g.:

servers:
  - url: "{apiRoot}/qod/v0"
    variables:
      apiRoot:
        default: http://localhost:9091
        description: API root
@jlurien jlurien added the enhancement New feature or request label Oct 6, 2023
@eric-murray
Copy link
Collaborator

Hi @jlurien

I agree with you, but ...

There is a specific issue for v0 variants in that successive versions may include breaking changes (anything can change at any time). We appear to be moving to a solution where we keep semantic versioning for the version number (so major version number remains 0), but use a different value in the path that changes with any breaking changes (hopefully any minor version increment).

If a common naming convention was adopted for this revised basepath naming was adopted, then the above scheme would work. But the workaround is just for implementors to go and change it anyway to a value of their choice, in which case the /v0 part of the basepath needs to remain a variable.

This being discussed as part of the release management issue, so that needs to be concluded before we can take this proposal to PR.

@hdamker hdamker transferred this issue from camaraproject/Commonalities Dec 6, 2023
@tanjadegroot
Copy link
Contributor

The proposed closure is to

  1. add @jlurien proposed servers definition to the API guidelines in commonalities as per above:
servers:
  - url: "{apiRoot}/qod/v0"
    variables:
      apiRoot:
        default: http://localhost:9091
        description: API root
  1. Have a CAMARA specific exception for initial public-release APIs with API version with x=0 which may have v0.y in the base path.
    It should be understood that any v0 API has a risk to change !

example

servers:
  - url: "{apiRoot}/qod/v0.10"

This is described in the API versioning here: API versioning

With that I think this issue can be transferred back to Commonalities to update the API guidelines with the above information.

@hdamker hdamker transferred this issue from camaraproject/ReleaseManagement May 14, 2024
@hdamker
Copy link
Collaborator

hdamker commented May 14, 2024

Transferred back to Commonalities as agreed within https://wiki.camaraproject.org/display/CAM/2024-05-14+Release+WG+Minutes, as the changes have to done in Commonalities.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request release management
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants