microsoft / kiota

OpenAPI based HTTP Client code generator
https://aka.ms/kiota/docs
MIT License
2.82k stars 191 forks source link

Generate models for all components in swagger.json #4835

Open IkeOTL opened 2 months ago

IkeOTL commented 2 months ago

Is your feature request related to a problem? Please describe the problem.

I'm automating client generation for my APIs. However, not all models that are shared my services are directly used by endpoints, so Kiota isn't generating the models for them.

Client library/SDK language

Csharp

Describe the solution you'd like

Add a flag on kiota generate --all-models to enable generating models for all components in the swagger.json.

Additional context

I'm generating my swagger.json using Swashbuckle.Swaggergen and adding in the models that aren't in endpoints using DocumentFilters

services.AddSwaggerGen(c =>
{
    c.DocumentFilter<ServiceDocumentFilter<AudioReadyEvent>>();
});
baywet commented 2 months ago

Hi @IkeOTL Thanks for using kiota and for reaching out. As your rightly identified, trimming isolated models is a feature of kiota. Can you provide more context on why you'd like models that are in essence "useless to the client" to be generated anyway?

IkeOTL commented 2 months ago

Hi @baywet In my use case I have several API services and they all communicate with each other. However, they don't always interact over HTTP APIs, a lot of the services are queue based, or event driven, and I would like for the classes used in the messages to be included in my auto-generated client pipeline so I don't need to copy the classes in multiple projects.

Or maybe I'm going about this the wrong way, do you have other suggestions?

baywet commented 2 months ago

Thanks for the additional context Are those models for webhooks by any chance? As this is something we plan to add support for #3914

I wouldn't say it's the wrong way, but maybe an unintended way. When we started out kiota the focus was very much clients for REST/HTTP APIs. Other protocols were never in scope. You might be able to generate those additional models, but are they going to serialize properly for the desired protocol? etc.

IkeOTL commented 2 months ago

Currently not webhooks. An example would be serializimg an object to JSON and sending it as message to an AWS SQS queue from one service, and polled from another service where it will deserialize into the object. And since I publish my client package in CI/CD it would be nice to have Kiota also harvest these objects from the swagger file since they're already there.

I used to use NSwag, which did this, but I'd rather not use that anymore as it seems less future proof than Kiota

baywet commented 2 months ago

Thanks for the additional context. It's the first time we hear about using the generated models outside of an HTTP context. We usually refrain from adding new command line switches whenever we can to keep the tool simple. The alternatives being to use a environment variable (not great for "update" scenarios), remove trimming all together (big part of the kiota proposition) or not make this change at all (workaround could be to add "dummy" endpoints to the description).

I'll defer to @sebastienlevert on this one as the program manager.

sebastienlevert commented 2 months ago

@baywet can you expand on the "update" scenarios and why it's not great? While this is a value of kiota (to trim components), it's something that brings a behavior that might be unexpected. Having something to turn it off would provide the capability while not breaking anybody. Now, environment variables vs. switches, I feel the problem is the same as now we're having bits of the customization on switches, and other bits in environment variables. To stay consistent, I would suggest we stick to a single customization option.

baywet commented 2 months ago

@sebastienlevert it would be a problem for updates if we went the variable route.

  1. Set the variable
  2. Generate a client
  3. Reopen terminal
  4. Update the client

Now all of a sudden a bunch of classes just disappeared.

sebastienlevert commented 2 months ago

Yeah, we lose on idempotency because we wouldn't persist this, good point. So the CLI switch is really the only option...

baywet commented 2 months ago

Assuming we went down that road, what would we call this switch? --no-trimming ? any better idea?

andrueastman commented 2 months ago

Maybe --include-all-components as what we're excluding are openApi components? We could have a possible future where we add a feature to exclude/trim other stuff so it may be useful to be specific.

baywet commented 2 months ago

what about --include-all-models then?

andrueastman commented 2 months ago

what about --include-all-models then?

+1

baywet commented 2 months ago

@maisarissi since Seb is out, and since you've worked a lot with the CLI lately, what are your thoughts? Are we good to proceed with this name and provide guidance to @IkeOTL so they can implement the change?

maisarissi commented 2 months ago

I have never thought about an use case when one might want to generate all models, thanks for sharing @IkeOTL ! I'm in favor of the switch approach and since we have introduced the models concept, I like the --include-all-models one. I am assuming that using --include-all-models would generate all inline models as well as all schema components no matter the endpoint selection.

baywet commented 2 months ago

I don't see a benefit of generating the inline models for endpoints that are not selected. By nature, they are not reusable.

maisarissi commented 2 months ago

Yes, I do agree that there is no benefit of generating inline models for endpoints that are not selected, I was just thinking on semantics as we are saying "include all models" and it's not actually all of them. But I wasn't able to come up with a better naming. Maybe --include-all-schemas, --include-schema-models, --all-schema-models, --include-defined-models?

baywet commented 2 months ago

I mean, a complete name would be either --include-all-component-schemas or --include-all-reusable-models but I fear this is going to create more confusion than anything.

sebastienlevert commented 1 month ago

Based on today's discussion, the decision is to build a new switch and offer a wildcard AND specific components. The components would also automatically bring their dependent components.

kiota generate ... Will trigger trimming and will generate components connected to paths kiota generate ... --include-components * Will not trigger trimming and will generate ALL components, with or without connect paths kiota generate ... --include-components user --include-components team --include-components ... Will not trigger trimming and will generate only the specified component and their dependencies.

A component name that doesn't exist should make this WARN.