Skip to content

Conversation

@wing328
Copy link
Member

@wing328 wing328 commented Mar 11, 2019

PR checklist

  • Read the contribution guidelines.
  • Ran the shell script under ./bin/ to update Petstore sample so that CIs can verify the change. (For instance, only need to run ./bin/{LANG}-petstore.sh, ./bin/openapi3/{LANG}-petstore.sh, ./bin/security/{LANG}-petstore.sh and ./bin/openapi3/security/{LANG}-petstore.sh if updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in .\bin\windows\.
  • Filed the PR against the correct branch: master, 3.4.x, 4.0.x. Default: master.
  • Copied the technical committee to review the pull request if your PR is targeting a particular programming language.

Description of the PR

Renamed scala-httpclient to scala-httpclient-deprecated. Users should switch to scala-akka instead.

cc @clasnake , @jimschubert @shijinkui, @ramzimaalej

@wing328 wing328 added this to the 4.0.0 milestone Mar 11, 2019
@ramzimaalej
Copy link
Contributor

LGTM

@wing328 wing328 merged commit b128d14 into master Mar 11, 2019
@wing328 wing328 deleted the scala-http-client-deco branch March 11, 2019 14:45
@jimschubert
Copy link
Member

@wing328 Can you comment a bit on the intention of the PR, such as why deprecate scala-httpclient rather than scala-akka (or, at all)? I'm confused as to why we would force Scala consumers to pull in Akka rather than a less bloated (i..e smaller/less-opinionated?) http client, but I might be missing a connecting issue or discussion.

I think it would make sense to have just a scala client
generator with a library option that includes akka/http/etc. client packages, and I'm wondering if this is a first step toward such a goal.

As an example use case (and why I all about intention): I use the scala-httpclient generator at work, specifically because I need a Scala client with custom templates. But, the scala-akka generator includes lots of akka specific files that we don't need or use and would cause a compilation error if not overridden. I assume that like my team, others will see a deprecation notice and switch from scala-httpclient to scala-akka thinking there's an improved use case. In my team's case, we aren't able to use akka within our project. We could, however, use scala-httpclient. We generate a custom client for Finagle http, so we can reuse Finagle modules we'd defined elsewhere in our stack.

For reference those additional files in scala-akka would be ApiInvoker.scala, ApiRequest.scala, ApiSettings.scala, and EnumSerializers.scala. The scala-httpclient generator is simpler to customize because of the fewer supporting files (which my team does use).

@wing328
Copy link
Member Author

wing328 commented Mar 12, 2019

Before this PR, we already marked the scala-httpclient as deprecated in the generator's help text: https://github.com/OpenAPITools/openapi-generator/blob/master/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/ScalaHttpClientCodegen.java#L222

The main reason is that the underlying http lib is no longer maintained. No update since May 2016: https://github.com/swagger-api/swagger-async-httpclient. scala-akka is the only choice remaining and that's why we've been asking people to use scala-akka instead.

Thanks for sharing more about your use case. I can certainly undo the changes but I think we really need to replace swagger-async-httpclient with some other http libraries that are better maintained.

@jimschubert
Copy link
Member

@wing328 thanks for the details. I'll look into creating a scala-finagle generator to replace this deprecated generator.

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