Skip to content

operationId should be optional #92

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

Closed
ghost opened this issue Jul 15, 2020 · 4 comments
Closed

operationId should be optional #92

ghost opened this issue Jul 15, 2020 · 4 comments
Labels
✨ enhancement New feature or improvement

Comments

@ghost
Copy link

ghost commented Jul 15, 2020

As per swagger's doc, operationId is an optional parameter:

operationId is an optional unique string used to identify an operation. If provided, these IDs must be unique among all operations described in your API.

Therefore, the following line should use a .get("operationId"):
https://github.com/triaxtec/openapi-python-client/blob/7ef8b93931a33829de2e272f530ac379fc976670/openapi_python_client/openapi_parser/openapi.py#L146

@dbanty
Copy link
Collaborator

dbanty commented Jul 15, 2020

We'll just need to figure out what the name the generated functions when there is not operationId. Any suggestions? I think most other tools do something with combining the method and path, where GET /my/endpoint would become something like get_my_endpoint. Not sure what to do with path parameters though (GET /my/endpoint/{id: int}).

Any suggestions on what you'd like to see for automatic naming?

@dbanty dbanty added the ✨ enhancement New feature or improvement label Jul 21, 2020
@dtkav
Copy link
Contributor

dtkav commented Sep 4, 2020

Connexion is a server-side project, but here's an example of automatic naming using the path / method.
https://github.com/zalando/connexion/blob/master/connexion/resolver.py#L69-L135

@dbanty
Copy link
Collaborator

dbanty commented Sep 8, 2020

Thanks @dtkav! For some reason that link is displaying in my browser with all the leading whitespace stripped which makes it hard to read- but it seems like the basics are:

  1. Split the path parts on /
  2. Remove any variables {}
  3. Replace "-" with "_"
  4. Join back up the parts with "."
  5. Append the REST method

I think I'm missing a bit of context- it's clearly doing something differently if the path ends in a var but some of the logic is based around some defaults I think?

FastAPI does something similar but the name param comes from the name of the function/class, which obviously we don't have.

I believe this is the code that openapi-generator uses. I think if we follow that same idea then GET /my/endpoint/{id} would become my_endpoint_id_get which would conflict with GET /my/endpoint/id. Maybe that's not the end of the world and we just catch conflicting names and emit a warning about them.

@dbanty
Copy link
Collaborator

dbanty commented Sep 26, 2020

Added auto name generation roughly matching above, though the method is before the path instead of after since that makes more sense to me. If you have GET /path/with/{param}/ it will produce a module with the name "get_path_with_param".

This will be available in 0.6.1 later today.

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

No branches or pull requests

2 participants