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 module with Play 2.9 support #3313

Merged
merged 11 commits into from
Nov 30, 2023
Merged

Conversation

cptwunderlich
Copy link
Contributor

In this PR, Play was bumped up to version 3, which switches from Akka to Pekko.
However, we were hoping to go to Play 2.9 first.

In this PR, I simply created 2.9 versions for everything regarding Play.

Please let me know, if anything else is needed.

To expose endpoint as a [play-server](https://www.playframework.com/) first add the following dependencies:

```scala
"com.softwaremill.sttp.tapir" %% "tapir-play-server" % "@VERSION@"
Copy link
Member

Choose a reason for hiding this comment

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

I think this would need different artifact names (with 29)?
But maybe we could have a single documentation page for JSON, I think the only difference would be the name of the artifact - so we could simply say that if you are on 3.0, use this artifact, if you are on 2.9, use this other one

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I fixed that and also link to the 2.8 docs I found.

@adamw
Copy link
Member

adamw commented Nov 22, 2023

Thanks! Are there any code differences between the codebases, or is it a direct copy, just different dependencies? If it's a direct copy, then maybe we could simply have the subproject use the play sources, but with different libraryDependencies?

@cptwunderlich
Copy link
Contributor Author

So, the client and server code differs in a tiny way: The imports are for akka, not pekko.

The json-play code is exactly the same - that does bother me and I tried to use a single source dir for the build, however, my sbt-foo is not great and I can't figure out how to correctly change the output directory (as to get rid of the resulting error "Overlapping output directories").

@kciesielski
Copy link
Member

@Mergifyio update

Copy link
Contributor

mergify bot commented Nov 22, 2023

update

❌ Mergify doesn't have permission to update

For security reasons, Mergify can't update this pull request. Try updating locally.
GitHub response: refusing to allow a GitHub App to create or update workflow .github/workflows/ci.yml without workflows permission

@kciesielski
Copy link
Member

@cptwunderlich Thanks a lot for this PR! I pushed a few fixes to the documentation. We can't have both dependency on playServer and playServer29 there, because these modules import classes with clashing names. I skipped one scala mdoc:compile-only in the 2.9 documentation, the rest compiles well.
As for the json-play module, I'll check later how to deal with shared sources.

@kciesielski
Copy link
Member

@Mergifyio update

Copy link
Contributor

mergify bot commented Nov 23, 2023

update

✅ Branch has been successfully updated

@kciesielski
Copy link
Member

I added some more fixes and it seems to be fine now. json/play29json uses sources from json/play-json.
If you don't have any more comments @cptwunderlich @adamw, we're good to go.

Add the dependency:

```scala
"com.softwaremill.sttp.tapir" %% "tapir-play-client" % "@VERSION@"
Copy link
Member

Choose a reason for hiding this comment

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

the artifact name here is wrong - I think it would really be better to combine the docs into a single page, just with mentioning alternative artifact names (plus one or two akka/pekko imports) - these are all the differences

@cptwunderlich
Copy link
Contributor Author

@adamw and @kciesielski thank you very much for your support!

@adamw I have merged the docs for Play 3 and 2.9 as you suggested.

@adamw
Copy link
Member

adamw commented Nov 24, 2023

@cptwunderlich Perfect, thanks! :)

@adamw
Copy link
Member

adamw commented Nov 27, 2023

@Mergifyio rebase

Copy link
Contributor

mergify bot commented Nov 27, 2023

rebase

✅ Branch has been successfully rebased

@adamw
Copy link
Member

adamw commented Nov 29, 2023

@Mergifyio rebase

Copy link
Contributor

mergify bot commented Nov 29, 2023

rebase

✅ Branch has been successfully rebased

@adamw
Copy link
Member

adamw commented Nov 29, 2023

https://github.com/Mergifyio rebase

Copy link
Contributor

mergify bot commented Nov 29, 2023

rebase

✅ Nothing to do for rebase action

@adamw
Copy link
Member

adamw commented Nov 29, 2023

We're getting some Scala 3 errors ... I'll try rebasing to get the new test reports

@adamw
Copy link
Member

adamw commented Nov 29, 2023

@Mergifyio rebase

Copy link
Contributor

mergify bot commented Nov 29, 2023

rebase

✅ Nothing to do for rebase action

@kciesielski
Copy link
Member

@adamw I tried to retry Scala 3 tests quite a few times, looks like they just freeze at some point. There were no failures in logs. I haven't dive deeper to isolate what's actually freezing, possibly integration tests for play29-server.

@kciesielski
Copy link
Member

I reverted Scala 3 support, it requires some extra tinkering with Akka versions. Tapir didn't have Scala 3 support for Play 2 before, and we treat Play 2.9 as a temporary server for those who are about to migrate to 3.x, so I'll just reduce to Scala 2 for now.
https://www.playframework.com/documentation/2.9.x/ScalaAkka#Important-note-on-using-Akka-HTTP-10.5.0-or-newer-with-Scala-3

@kciesielski
Copy link
Member

@Mergifyio rebase

Copy link
Contributor

mergify bot commented Nov 30, 2023

rebase

✅ Branch has been successfully rebased

@kciesielski kciesielski merged commit 9ca2968 into softwaremill:master Nov 30, 2023
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.

3 participants