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-sttp: fix for issue 15785 api returns unit. #18537

Merged

Conversation

dwischolek-modulon
Copy link
Contributor

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/configs/*.yaml
    ./bin/utils/export_docs_generators.sh
    

This PR solves #15785 for the scala-sttp (3) generator.
Rationale for the changes:

  • the problem that was tackled in Fixed scala-sttp generator #11949 was not really about default values.
  • asUnit[Unit] leads to an error, anyways, so not only with default responses, this is the wrong encoding.
  • using asEither(asString, ignore) works, but turns the structured error in Left into a String, shedding information in the process. The "asString.mapWithMetadata(..."-construct allows for empty or garbage response bodies while keeping the Left() information intact.
  • asJsonAlwaysUnsafe is gone from sttp for a LONG time. This needed replacement, anyways.

@wing328
Copy link
Member

wing328 commented Apr 30, 2024

thanks for the PR

cc
@clasnake (2017/07), @jimschubert (2017/09) ❤️, @shijinkui (2018/01), @ramzimaalej (2018/03), @chameleon82 (2020/03), @Bouillie (2020/04) @Fish86 (2023/06)

@dwischolek-modulon
Copy link
Contributor Author

dwischolek-modulon commented Apr 30, 2024

@wing328 I'm not sure if you had seen it, but #18536 is a different commit with changes for sttp4. This one is sttp3 only. I split those up in case one or the other needed a roll back.
Edit: Meaning the other one should not be closed, maybe.

@wing328
Copy link
Member

wing328 commented Apr 30, 2024

ah right. sorry. just reopened it

@wing328 wing328 merged commit afd3a78 into OpenAPITools:master May 1, 2024
25 checks passed
@dwischolek-modulon dwischolek-modulon deleted the fix-for-issue-15785-sttp branch June 21, 2024 14:33
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

2 participants