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

[scala][client] invoker package enhancement #9381

Merged
merged 2 commits into from
Jun 18, 2021

Conversation

sandeepjindal
Copy link
Contributor

@sandeepjindal sandeepjindal commented May 1, 2021

Here are the details in the issue: #9378

Steps to verify the change run these commands:

  • ./mvnw clean package
  • ./bin/generate-samples.sh bin/configs/scala-legacy.yaml

you should see the new invoker pacakge like this:
image

PR checklist

  • Read the contribution guidelines.
  • Pull Request title clearly describes the work in the pull request and Pull Request description provides details about how to validate the work. Missing information here may result in delayed response from the community.
  • Run the following to build the project and update samples:
    ./mvnw clean package 
    ./bin/generate-samples.sh
    ./bin/utils/export_docs_generators.sh
    
    Commit all changed files.
    This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
    These must match the expectations made by your contribution.
    You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example ./bin/generate-samples.sh bin/configs/java*.
    For Windows users, please run the script in Git BASH.
  • File the PR against the correct branch: master, 5.1.x, 6.0.x
    @mention the@clasnake (2017/07), @jimschubert (2017/09) ❤️, @shijinkui (2018/01), @ramzimaalej (2018/03), @chameleon82 (2020/03), @Bouillie (2020/04)

@sandeepjindal sandeepjindal changed the title updated invoker [scala][client] invoker package enhancement May 1, 2021
Copy link
Contributor

@chameleon82 chameleon82 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR @sandeepjindal.
Please ensure for most of scala clients invokerPackage by default is mainPackage + ".core" this PR will not override default behavior.
Please run the script ./bin/generate-samples.sh to regenerate all scala clients

@sandeepjindal
Copy link
Contributor Author

sandeepjindal commented May 1, 2021

Please ensure for most of scala clients invokerPackage by default is mainPackage + ".core" this PR will not override default behavior.

Only below marked language framework is using invokerPackage should i check in these only:
image

@sandeepjindal
Copy link
Contributor Author

@chameleon82 Thanks for the help i have pushed the fix, please help review and approve.

additionalProperties:
apiPackage: org.openapitools.example.api
invokerPackage: org.openapitools.example.invoker
artifactId: scala-legacy-petstore
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sandeepjindal Please run and commit those files

./bin/generate-samples.sh bin/configs/scala-legacy.yaml

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done please check

@sandeepjindal
Copy link
Contributor Author

Maintainers:
@clasnake (2017/07), @jimschubert (2017/09) ❤️, @shijinkui (2018/01), @ramzimaalej (2018/03), @chameleon82 (2020/03), @Bouillie (2020/04)
Please help merge.

@wing328
Copy link
Member

wing328 commented May 12, 2021

I ran sbt test on samples/client/petstore/scala-httpclient-deprecated but got the following errors:

[info] Non-compiled module 'compiler-bridge_2.11' for Scala 2.11.12. Compiling...
[info]   Compilation completed in 8.364s.
[error] /Users/williamcheng/Code/openapi-generator/samples/client/petstore/scala-httpclient-deprecated/src/main/scala/org/openapitools/example/invoker/ApiInvoker.scala:40:8: ScalaJsonUtil is already defined as object ScalaJsonUtil
[error] object ScalaJsonUtil {
[error]        ^
[error] /Users/williamcheng/Code/openapi-generator/samples/client/petstore/scala-httpclient-deprecated/src/main/scala/org/openapitools/example/invoker/ApiInvoker.scala:54:7: ApiInvoker is already defined as class ApiInvoker
[error] class ApiInvoker(val mapper: ObjectMapper = ScalaJsonUtil.getJsonMapper,
[error]       ^
[error] /Users/williamcheng/Code/openapi-generator/samples/client/petstore/scala-httpclient-deprecated/src/main/scala/org/openapitools/example/invoker/ApiInvoker.scala:237:7: ApiException is already defined as class ApiException
[error] class ApiException(val code: Int, msg: String) extends RuntimeException(msg)
[error]       ^
[error] /Users/williamcheng/Code/openapi-generator/samples/client/petstore/scala-httpclient-deprecated/src/main/scala/org/openapitools/example/invoker/AsyncClient.scala:9:7: AsyncClient is already defined as class AsyncClient
[error] class AsyncClient(config: SwaggerConfig) extends Closeable {
[error]       ^
[error] four errors found
[error] (Compile / compileIncremental) Compilation failed
[error] Total time: 21 s, completed 12 May 2021, 10:27:43 pm

Does it work for you in your machine?

@sandeepjindal
Copy link
Contributor Author

I ran sbt test on samples/client/petstore/scala-httpclient-deprecated but got the following errors:

[info] Non-compiled module 'compiler-bridge_2.11' for Scala 2.11.12. Compiling...
[info]   Compilation completed in 8.364s.
[error] /Users/williamcheng/Code/openapi-generator/samples/client/petstore/scala-httpclient-deprecated/src/main/scala/org/openapitools/example/invoker/ApiInvoker.scala:40:8: ScalaJsonUtil is already defined as object ScalaJsonUtil
[error] object ScalaJsonUtil {
[error]        ^
[error] /Users/williamcheng/Code/openapi-generator/samples/client/petstore/scala-httpclient-deprecated/src/main/scala/org/openapitools/example/invoker/ApiInvoker.scala:54:7: ApiInvoker is already defined as class ApiInvoker
[error] class ApiInvoker(val mapper: ObjectMapper = ScalaJsonUtil.getJsonMapper,
[error]       ^
[error] /Users/williamcheng/Code/openapi-generator/samples/client/petstore/scala-httpclient-deprecated/src/main/scala/org/openapitools/example/invoker/ApiInvoker.scala:237:7: ApiException is already defined as class ApiException
[error] class ApiException(val code: Int, msg: String) extends RuntimeException(msg)
[error]       ^
[error] /Users/williamcheng/Code/openapi-generator/samples/client/petstore/scala-httpclient-deprecated/src/main/scala/org/openapitools/example/invoker/AsyncClient.scala:9:7: AsyncClient is already defined as class AsyncClient
[error] class AsyncClient(config: SwaggerConfig) extends Closeable {
[error]       ^
[error] four errors found
[error] (Compile / compileIncremental) Compilation failed
[error] Total time: 21 s, completed 12 May 2021, 10:27:43 pm

Does it work for you in your machine?

Thanks for notifying, removed duplicated files.

remove unwanted set the invoker

fixed the ordering of supported files

added sample generated files

regenerated samples

added docs
@sandeepjindal
Copy link
Contributor Author

Maintainers:
@clasnake (2017/07), @jimschubert (2017/09) ❤️, @shijinkui (2018/01), @ramzimaalej (2018/03), @chameleon82 (2020/03), @Bouillie (2020/04)

please help review.
As checked the failure is not related with branch changes

@sandeepjindal
Copy link
Contributor Author

Maintainers:
@clasnake (2017/07), @jimschubert (2017/09) ❤️, @shijinkui (2018/01), @ramzimaalej (2018/03), @chameleon82 (2020/03), @Bouillie (2020/04)

please help review.
As checked the failure is not related with branch changes

May i please followup on this

@chameleon82
Copy link
Contributor

@wing328 could you help to check circleci build? seems it is not pr changes related fail

@sandeepjindal
Copy link
Contributor Author

@wing328 can i please followup on this

@wing328
Copy link
Member

wing328 commented Jun 18, 2021

CircleCI failure already fixed in the master.

@wing328
Copy link
Member

wing328 commented Jun 18, 2021

Tested samples/client/petstore/scala-httpclient-deprecated again and no longer seeing the same issue:

[warn] There may be incompatibilities among your library dependencies; run 'evicted' to see detailed eviction warnings.
[info] Compiling 11 Scala sources to /home/wing328/Code/openapi-generator/samples/client/petstore/scala-httpclient-deprecated/target/scala-2.11/classes ...
[info] Non-compiled module 'compiler-bridge_2.11' for Scala 2.11.12. Compiling...
[info]   Compilation completed in 32.052s.
[info] Done compiling.
WARNING: An illegal reflective access operation has occurred
WARNING: Illegal reflective access by com.google.protobuf.UnsafeUtil (file:/home/wing328/.sbt/boot/scala-2.12.7/org.scala-sbt/sbt/1.2.8/protobuf-java-3.3.1.jar) to field java.nio.Buffer.address
WARNING: Please consider reporting this to the maintainers of com.google.protobuf.UnsafeUtil
WARNING: Use --illegal-access=warn to enable warnings of further illegal reflective access operations
WARNING: All illegal access operations will be denied in a future release
[info] Run completed in 143 milliseconds.
[info] Total number of tests run: 0
[info] Suites: completed 0, aborted 0
[info] Tests: succeeded 0, failed 0, canceled 0, ignored 0, pending 0
[info] No tests were executed.
[success] Total time: 194 s, completed Jun 18, 2021, 3:49:24 PM

@wing328 wing328 merged commit c7038d1 into OpenAPITools:master Jun 18, 2021
@wing328 wing328 added this to the 5.2.0 milestone Jun 18, 2021
@sandeepjindal
Copy link
Contributor Author

Thanks appreciated @wing328 @chameleon82

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

Successfully merging this pull request may close these issues.

None yet

3 participants