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

Generate models for all components in swagger.json #4835

Open
IkeOTL opened this issue Jun 16, 2024 · 18 comments
Open

Generate models for all components in swagger.json #4835

IkeOTL opened this issue Jun 16, 2024 · 18 comments
Labels
Csharp Pull requests that update .net code needs more information type:feature New experience request
Milestone

Comments

@IkeOTL
Copy link

IkeOTL commented Jun 16, 2024

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>>();
});
@IkeOTL IkeOTL added status:waiting-for-triage An issue that is yet to be reviewed or assigned type:feature New experience request labels Jun 16, 2024
@github-project-automation github-project-automation bot moved this to Needs Triage 🔍 in Kiota Jun 16, 2024
@baywet
Copy link
Member

baywet commented Jun 17, 2024

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?

@baywet baywet added status:waiting-for-author-feedback Issue that we've responded but needs author feedback to close and removed status:waiting-for-triage An issue that is yet to be reviewed or assigned labels Jun 17, 2024
@baywet baywet moved this from Needs Triage 🔍 to Waits for author 🔁 in Kiota Jun 17, 2024
@baywet baywet added this to the Backlog milestone Jun 17, 2024
@IkeOTL
Copy link
Author

IkeOTL commented Jun 17, 2024

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?

@microsoft-github-policy-service microsoft-github-policy-service bot added Needs: Attention 👋 and removed status:waiting-for-author-feedback Issue that we've responded but needs author feedback to close labels Jun 17, 2024
@baywet
Copy link
Member

baywet commented Jun 17, 2024

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.

@baywet baywet added status:waiting-for-author-feedback Issue that we've responded but needs author feedback to close and removed Needs: Attention 👋 labels Jun 17, 2024
@IkeOTL
Copy link
Author

IkeOTL commented Jun 17, 2024

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

@microsoft-github-policy-service microsoft-github-policy-service bot added Needs: Attention 👋 and removed status:waiting-for-author-feedback Issue that we've responded but needs author feedback to close labels Jun 17, 2024
@msgraph-bot msgraph-bot bot added the Csharp Pull requests that update .net code label Jun 19, 2024
@baywet
Copy link
Member

baywet commented Jun 19, 2024

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.

@baywet baywet moved this from Waits for author 🔁 to In Design 🎨 in Kiota Jun 19, 2024
@sebastienlevert
Copy link
Contributor

@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
Copy link
Member

baywet commented Jun 19, 2024

@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
Copy link
Contributor

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

@baywet
Copy link
Member

baywet commented Jun 20, 2024

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

@andrueastman
Copy link
Member

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
Copy link
Member

baywet commented Jun 25, 2024

what about --include-all-models then?

@andrueastman
Copy link
Member

what about --include-all-models then?

+1

@baywet
Copy link
Member

baywet commented Jun 25, 2024

@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
Copy link
Contributor

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
Copy link
Member

baywet commented Jun 25, 2024

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

@maisarissi
Copy link
Contributor

maisarissi commented Jun 26, 2024

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
Copy link
Member

baywet commented Jun 26, 2024

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
Copy link
Contributor

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.

@sebastienlevert sebastienlevert moved this from In Design 🎨 to Todo 📃 in Kiota Jul 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Csharp Pull requests that update .net code needs more information type:feature New experience request
Projects
Status: New📃
Development

No branches or pull requests

5 participants