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

Add scala3 support for spray json #2181

Merged
merged 2 commits into from
May 20, 2024

Conversation

MichelEdkrantz
Copy link
Contributor

Before submitting pull request:

  • Check if the project compiles by running sbt compile
  • Verify docs compilation by running sbt compileDocs
  • Check if tests pass by running sbt test
  • Format code by running sbt scalafmt

Tests could not complete because of OOM issues. Just ran this with standard settings on an M3 Macbook. Any common tips or tricks used here?

Running sbtformat yielded format changed for files i did not touch.

modified:   core/src/main/scala/sttp/client4/testing/ResponseStub.scala
	modified:   observability/opentelemetry-metrics-backend/src/main/scala/sttp/client4/opentelemetry/OpenTelemetryMetricsBackend.scala
	modified:   observability/opentelemetry-metrics-backend/src/main/scala/sttp/client4/opentelemetry/OpenTelemetryMetricsConfig.scala
	modified:   observability/prometheus-backend/src/main/scala/sttp/client4/prometheus/PrometheusBackend.scala

@MichelEdkrantz
Copy link
Contributor Author

Discussed here #2179

@adamw
Copy link
Member

adamw commented May 16, 2024

Thanks!

Testing is a problem, due to Scala.JS+Chrome, which doesn't release memory or something like that. It's a problem known for a couple of years without a resolution ;) There's a work-around in the readme, bascially you need to run only a subset of tests:

Note that running the default test task will run the tests using both the JVM and JS backends, and is likely to run out of memory. If you'd like to run the tests using only the JVM backend, execute: sbt rootJVM/test.

But if the code compiles, that 90% of success ;)

As for formatting - yes, we should get an additional build step which verifies that files are properly formatted, although nobody got to it yet ;)

@adamw
Copy link
Member

adamw commented May 16, 2024

It seems there's a compilation error:

[error] -- [E172] Type Error: /home/runner/work/sttp/sttp/json/spray-json/src/test/scala/sttp/client4/SprayJsonTests.scala:85:105 
[error] 85 |    val request: Request[Either[String, String]] = basicRequest.get(Uri("http://example.org/")).body(json)
[error]    |                                                                                                         ^
[error]    |No given instance of type sttp.client4.BodySerializer[spray.json.JsObject] was found for a context parameter of method body in trait PartialRequestExtensions.
[error]    |I found:
[error]    |
[error]    |    sttp.client4.sprayJson.sprayBodySerializer[B]()
[error]    |
[error]    |But method sprayBodySerializer in trait SttpSprayJsonApi does not match type sttp.client4.BodySerializer[spray.json.JsObject].
[error] one error found

@MichelEdkrantz
Copy link
Contributor Author

MichelEdkrantz commented May 18, 2024

Hi,
sorry about the initial state and thanks for the patience. This was a good learning experience for me, having very little experience with both sbt and cross-builds. It was not obvious to me at first that scala3 did not run locally.

It seems, this can be resolved by adding this implicit for scala3

implicit private val jsObjectSerializer: BodySerializer[JsObject] = sprayBodySerializer(RootJsObjectFormat)

This build is now ✅
https://github.com/MichelEdkrantz/sttp/actions/runs/9137873805

I did another branch forking from 3.9.6 for the client3, without these issues.
https://github.com/MichelEdkrantz/sttp/actions/runs/9128354147

@adamw
Copy link
Member

adamw commented May 20, 2024

scala3 should run locally :) Are you compiling from the sbt console? It should be aggregated in the main build, but you can also switch projects (when in sbt console) by typing project sprayJson3 and then compile or test - to test only that simple project, using scala 3

@MichelEdkrantz
Copy link
Contributor Author

Thanks, yes, figured out how to run it locally with Scala 3 in the sbt console now

@adamw
Copy link
Member

adamw commented May 20, 2024

Ok, great, maybe we do need some better docs on how to do this :)

@adamw adamw merged commit 11b8b44 into softwaremill:master May 20, 2024
13 checks passed
@MichelEdkrantz MichelEdkrantz deleted the spray-json-scala3 branch May 20, 2024 09:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants