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

Extending/adding templates #171

Closed
pawamoy opened this issue Aug 28, 2020 · 2 comments
Closed

Extending/adding templates #171

pawamoy opened this issue Aug 28, 2020 · 2 comments
Labels
✨ enhancement New feature or improvement

Comments

@pawamoy
Copy link

pawamoy commented Aug 28, 2020

Is your feature request related to a problem? Please describe.
The OpenAPI specs I'm generating clients from are a bit convoluted. Lots of models with few differences, resulting in a lot of models submodules and classes. To make the clients more developer-friendly, I'm wrapping all their endpoints in one class each, flattening the arguments in each method.

A typical example:

class Datasets(_API):
    """The datasets API."""

    def create(self, solution_id: Optional[str] = None) -> Dataset:
        return datasets_api.create_dataset(client=self.client, json_body=DatasetCreate(solution_id=solution_id))

# _API is a base class setting self.client
# datasets_api is the generated datasets_client.api.default module
# Dataset and DatasetCreate are generated models

As you can see I'm simplifying the name and usage:

from datasets_client import Client
from datasets_client.api import default as datasets_api
from datasets_client.models import DatasetCreate

client = Client(...)
datasets_api.create_dataset(client, json_body=DatasetCreate(solution_id=solution_id))
datasets_api.other_method(client, *other_nested_args_and_models)

...becomes:

from datasets_client import Client, Datasets

datasets = Datasets(Client(...))
datasets.create(solution_id)  # don't pass client each time
datasets.other_method(*other_flat_args)

I can then wrap multiple classes like that into a main one:

class API:
    def __init__(self, base_url, token):
        self.base_url = base_url
        self.datasets = Datasets(
            client=AuthenticatedClient(base_url=self.base_url + "/rest-of-the-url", token=token)
        )
        self.other_api= OtherAPI(
            client=AuthenticatedClient(base_url=self.base_url + "/rest-of-the-other-url", token=token)
        )
        ...

In the end, a developer can just import and instantiate the API class, and start using every possible endpoint, without importing any model or endpoint themselves:

from project import API

api = API("http://example.com", "secret")
api.datasets.create(...)
api.other_api.check_status(...)

Anyway! (just explaning my use case)

Describe the solution you'd like
I'm not asking you to change how to generate clients. Rather, I'd like to be able to "add" templates to the generation process, so I could automatically (re)generate these wrapper classes of mine, using openapi-python-client's Jinja context. Sorry for the long introduction 😅

It could be done with a command line option to tell openapi-python-client to render every template within the specified folder into the generated client. The directory structure would be respected, allowing the user to "inject" new contents in any subfolder of the generated client.

Describe alternatives you've considered
Improve the OpenAPI specs I'm using, but it would result in breaking changes, so a lot more work for many other people in my team and other teams 😮

Additional context
Maybe what I'm doing here is plain wrong and I'm losing benefits of the generated clients layouts? Don't hesitate to share on this matter 😄

@pawamoy pawamoy added the ✨ enhancement New feature or improvement label Aug 28, 2020
@dbanty
Copy link
Collaborator

dbanty commented Aug 28, 2020

Trying to separate out some of the improvements you're making manually from some of the potential solutions so we can brainstorm about them individually (since some may be easier to solve than others). So it looks like the things you're doing are:

  1. Flattening argument models into just arguments. So instead of using a generated DatasetCreate class as an input, you're taking all the fields that would be in that class and using them as direct arguments to the function.
  2. Reducing calling boilerplate by storing repetitive values in a class. Specifically the client object.
  3. Combining multiple APIs into a single generated client

Does that about cover it?

As far as some possible solutions, I do like the idea of customizable templates but I think such a thing would be pretty far off. It would mean we need a stable documented API for how we render jinja templates- so this is probably a post-1.0 feature once the project itself is fairly stable. This would open up all sorts of doors though- even generating clients in different languages if we really wanted to. Of course then we're effectively becoming a full alternative to the Java openapi generator which I'm not sure is a responsibility I want 😅.

Flattening arguments in general feels like something we could achieve as a feature without too much work, though we'd have to be careful when there are other params (headers, query) which might have conflicting names.

The shared client in a class model is something I considered early on but ultimately decided on a more functional style for this project. This certainly has the downside of being more verbose. It does seem like a good idea to offer an alternative set of templates that's more object oriented for those who prefer that style. If we support more paradigms then we can lean more into each.

Combining multiple documents into one client is 100% something I want to do at some point. We use a lot of microservices which, like what you describe, often have a lot of overlap. I think it's going to take a lot of care to get this one right without generating errors all over the place. Maybe after my brain cools off a bit from the 0.6 refactor 🥵.

I believe this could be broken up into a few independent issues so we can bite off one at a time:

  1. Optionally flatten a single input body into kwarg instead of using a dataclass
  2. Support an object-oriented client style where API calls are methods.
  3. Support combining multiple OpenAPI documents into one client
  4. Stabilize template API to allow for customized templates

And of course anything I didn't notice as well 👍. Thanks for all your input, as always you give me a lot to think about.

@dbanty
Copy link
Collaborator

dbanty commented Nov 7, 2020

I think we have separate issues for each feature request in here now so I'm going to close this big one:

  1. Optionally flatten a single input body into kwarg instead of using a dataclass - I think this is something that should be done in an alternate template at least for now.
  2. Support an object-oriented client style where API calls are methods - Add object-oriented client option #224
  3. Support combining multiple OpenAPI documents into one client - #237
  4. Stabilize template API to allow for customized templates - support custom template path #231

@dbanty dbanty closed this as completed Nov 7, 2020
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