search

OpenAPITools / openapi-generator

OpenAPI Generator allows generation of API client libraries (SDK generation), server stubs, documentation and configuration automatically given an OpenAPI Spec (v2, v3)
https://openapi-generator.tech
Apache License 2.0
21.23k stars 6.43k forks source link

[REQ][csharp-netcore] httpclient library please remove old constructors #10001

Open Havunen opened 3 years ago

Havunen commented 3 years ago

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

I would like to have a setting to not create old constructors when generating client for csharp-netcore httpclient library. Currently I have large existing code base and we are migrating to .net core and same time converting our generated API clients to use http client based code. Because RestSharp in .net core may cause port exhaustion issue. Now when migrating the existing codebase it is difficult to find all the areas which need to be changed because the new generated code compiles the same as previous restsharp client did, however it would fail runtime to socket exhaustion issue.

Describe the solution you'd like

I would like to have an option or change http client library generator so that it does not generate constructors where its not required to give http client as parameter.

https://github.com/OpenAPITools/openapi-generator/blob/master/modules/openapi-generator/src/main/resources/csharp-netcore/libraries/httpclient/ApiClient.mustache#L192-L217

Describe alternatives you've considered

I have considered manually changing the generated code by commenting out these constructors but its problematic when we re-generate the code.

Blackclaws commented 2 years ago

Interesting request. Is this still an issue for you? We've deliberately kept in the old constructors so that its backwards compatible. Its not that hard to introduce a switch to get rid of them either. If you still have this as a problem we can consider changing that.

Havunen commented 2 years ago

Yes we do, we are currently using our custom fork of this open api generator. This is issue mostly because its difficult to find the wrong constructor usages in large codebase. Using them causes socket exhaustion bugs to us

Blackclaws commented 2 years ago

I see. Yeah this is indeed a problem in those cases. I'll see if I can't put in a pull request that makes those optional, maybe even by default.

wing328 commented 2 years ago

What about marking the constructor as deprecated to start with and then remove it in the 6.x release?

Blackclaws commented 2 years ago

That was my original thought yeah. We might also do that, deprecating them should at least throw warnings. Would that suffice @Havunen ?

Blackclaws commented 2 years ago

@Havunen I just checked and we do actually have the additionalProperty: isDeprecated.

Have you tried setting that to true and seeing if that already throws enough warnings for those constructors? The obsolete attribute is in the generator, it just isn't there by default.

devhl-labs commented 2 years ago

My pr adding a new library should help. #10627

Blackclaws commented 2 years ago

How does the PR help unless they switch over to that library?