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

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

Open
Havunen opened this issue Jul 21, 2021 · 8 comments
Open

Comments

@Havunen
Copy link
Contributor

Havunen commented Jul 21, 2021

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

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

Havunen commented Oct 18, 2021

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

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

wing328 commented Oct 18, 2021

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

@Blackclaws
Copy link
Contributor

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

@Blackclaws
Copy link
Contributor

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

My pr adding a new library should help. #10627

@Blackclaws
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants