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

Make the generated openapi spec more concrete and compliant to standard #1122

Closed
wants to merge 1 commit into from
Closed

Make the generated openapi spec more concrete and compliant to standard #1122

wants to merge 1 commit into from

Conversation

alikins
Copy link
Contributor

@alikins alikins commented Feb 11, 2021

ie, instead of {ansible_repository_href}, the
api urls are of form /pulp/api/v3/repositories/ansible/ansible/

Also makes the generated spec compliant to the openapi standard,
including not allowing endpoints to start with '{' and requiring
endpoints to start with '/'

Please be sure you have read our documentation on creating PRs:
https://docs.pulpproject.org/contributing/pull-request-walkthrough.html

@pulpbot
Copy link
Member

pulpbot commented Feb 11, 2021

WARNING!!! This PR is not attached to an issue. In most cases this is not advisable. Please see our PR docs for more information about how to attach this PR to an issue.

@daviddavis
Copy link
Contributor

I realize that our API is not compatible with the openapi spec but this change would fundamentally alter our API by instead having endpoints/bindings accept ids instead of hrefs (the former of which we're not exposing in responses). It might be better to raise this as a discussion on our mailing lists about the motivations for this change.

@bmbouter
Copy link
Member

Are we sure it's not compatible with the V3 standard? This is how the google APIs work for example, and there was an openapi schema issue they opened (I don't have the link handy) to have this be supported. I remember believing it wasn't compatible with V2, but it would be with V3.

ie, instead of {ansible_repository_href}, the
api urls are of form /pulp/api/v3/repositories/ansible/ansible/

Issue: no-issue
@alikins
Copy link
Contributor Author

alikins commented Feb 16, 2021

Another option would be to extract the endpoint/url/path generating bits from PulpSchemaGenerator.parse() to a method, and let galaxy_ng override it. Or maybe it an option or config setting etc.

@bmbouter
Copy link
Member

At the pulpcore meeting we looked at this PR and we can't accept it. Also we don't believe making it configurable is in the best interest of users. The reasons are:

  • The API uses hrefs and the openAPI schema description needs to describe it in the same way
  • The API itself can't be changed due to semver reasons

@bmbouter bmbouter closed this Mar 30, 2021
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.

None yet

4 participants