-
Notifications
You must be signed in to change notification settings - Fork 422
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
Migrate servers from tapir-loom #3304
Changes from 12 commits
e418a0e
76dfa35
25b4106
9a03a46
49af16b
1492132
a52779a
dcf8d22
8fe4593
a3f6643
ee87d44
e5ab8f2
10c944c
d33aff8
87235c3
5273d36
bc70301
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,31 +18,41 @@ jobs: | |
matrix: | ||
scala-version: [ "2.12", "2.13", "3" ] | ||
target-platform: [ "JVM", "JS", "Native" ] | ||
java: [ "11", "21" ] | ||
exclude: | ||
- scala-version: "2.12" | ||
target-platform: "Native" | ||
- scala-version: "2.12" | ||
java: "21" | ||
- scala-version: "2.13" | ||
target-platform: "Native" | ||
- java: "21" | ||
target-platform: "Native" | ||
- java: "21" | ||
target-platform: "JS" | ||
steps: | ||
- name: Checkout | ||
uses: actions/checkout@v3 | ||
- name: Set up JDK 11 | ||
- name: Set up JDK | ||
uses: actions/setup-java@v3 | ||
with: | ||
distribution: 'temurin' | ||
cache: 'sbt' | ||
java-version: 11 | ||
java-version: ${{ matrix.java }} | ||
- name: Install sam cli | ||
if: matrix.java == '11' | ||
run: | | ||
wget -q https://github.com/aws/aws-sam-cli/releases/latest/download/aws-sam-cli-linux-x86_64.zip | ||
unzip -q aws-sam-cli-linux-x86_64.zip -d sam-installation | ||
sudo ./sam-installation/install --update | ||
sam --version | ||
- name: Install NPM | ||
if: matrix.java == '11' | ||
run: | | ||
sudo apt install npm | ||
npm --version | ||
- name: Install AWS CDK | ||
if: matrix.java == '11' | ||
run: | | ||
npm install -g aws-cdk | ||
cdk --version | ||
|
@@ -52,10 +62,13 @@ jobs: | |
sudo apt-get update | ||
sudo apt-get install libidn2-dev libcurl3-dev | ||
echo "STTP_NATIVE=1" >> $GITHUB_ENV | ||
- name: Enable Loom-specific modules | ||
if: matrix.java == '21' | ||
run: echo "JDK_LOOM=1" >> $GITHUB_ENV | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. or we can just use https://stackoverflow.com/questions/2591083/getting-java-version-at-runtime in the sbt build? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This env variable makes the project build only jdk21 servers. Checking java version in sbt instead of reading it might cause confusion in local development. For example, I use 21 as my default working JDK, but I still want to build the "JDK11 set". More developers may have such a case. Maybe we should rename the variable to something meaning more "build loom servers only"? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. makes sense :) add this info to readme in |
||
- name: Compile | ||
run: sbt $SBT_JAVA_OPTS -v "compileScoped ${{ matrix.scala-version }} ${{ matrix.target-platform }}" | ||
- name: Compile documentation | ||
if: matrix.target-platform == 'JVM' | ||
if: matrix.target-platform == 'JVM' && matrix.java == '11' | ||
run: sbt $SBT_JAVA_OPTS -v compileDocumentation | ||
- name: Test | ||
if: matrix.target-platform != 'JS' | ||
|
@@ -109,21 +122,29 @@ jobs: | |
needs: [ci] | ||
if: github.event_name != 'pull_request' && (startsWith(github.ref, 'refs/tags/v')) | ||
runs-on: ubuntu-22.04 | ||
env: | ||
STTP_NATIVE: 1 | ||
strategy: | ||
matrix: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hopefully this will work and the two publishes will publish separate jars - no other way to find out than try ;) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, unfortunately there's no nice way to do a dry-run. We could probably make |
||
java: [ "11", "21" ] | ||
steps: | ||
- name: Checkout | ||
uses: actions/checkout@v3 | ||
- name: Set up JDK 11 | ||
- name: Set up JDK | ||
uses: actions/setup-java@v3 | ||
with: | ||
distribution: 'temurin' | ||
java-version: 11 | ||
java-version: ${{ matrix.java }} | ||
cache: 'sbt' | ||
- name: Install libidn2-dev libcurl3-dev | ||
if: matrix.java == '11' | ||
run: | | ||
sudo apt-get update | ||
sudo apt-get install libidn2-dev libcurl3-dev | ||
- name: Enable Native-specific modules | ||
if: matrix.java == '11' | ||
run: echo "STTP_NATIVE=1" >> $GITHUB_ENV | ||
- name: Enable Loom-specific modules | ||
if: matrix.java == '21' | ||
run: echo "JDK_LOOM=1" >> $GITHUB_ENV | ||
- name: Compile | ||
run: sbt $SBT_JAVA_OPTS compile | ||
- name: Publish artifacts | ||
|
@@ -134,12 +155,14 @@ jobs: | |
SONATYPE_USERNAME: ${{ secrets.SONATYPE_USERNAME }} | ||
SONATYPE_PASSWORD: ${{ secrets.SONATYPE_PASSWORD }} | ||
- name: Extract version from commit message | ||
if: matrix.java == '11' | ||
run: | | ||
version=${GITHUB_REF/refs\/tags\/v/} | ||
echo "VERSION=$version" >> $GITHUB_ENV | ||
env: | ||
COMMIT_MSG: ${{ github.event.head_commit.message }} | ||
- name: Publish release notes | ||
if: matrix.java == '11' | ||
uses: release-drafter/release-drafter@v5 | ||
with: | ||
config-name: release-drafter.yml | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -221,8 +221,10 @@ lazy val rawAllAggregates = core.projectRefs ++ | |
vertxServerZio1.projectRefs ++ | ||
jdkhttpServer.projectRefs ++ | ||
nettyServer.projectRefs ++ | ||
nettyServerLoom.projectRefs ++ | ||
nettyServerCats.projectRefs ++ | ||
nettyServerZio.projectRefs ++ | ||
nimaServer.projectRefs ++ | ||
zio1HttpServer.projectRefs ++ | ||
zioHttpServer.projectRefs ++ | ||
awsLambdaCore.projectRefs ++ | ||
|
@@ -251,13 +253,21 @@ lazy val rawAllAggregates = core.projectRefs ++ | |
awsCdk.projectRefs | ||
|
||
lazy val allAggregates: Seq[ProjectReference] = { | ||
if (sys.env.isDefinedAt("STTP_NATIVE")) { | ||
val filteredByNative = if (sys.env.isDefinedAt("STTP_NATIVE")) { | ||
println("[info] STTP_NATIVE defined, including native in the aggregate projects") | ||
rawAllAggregates | ||
} else { | ||
println("[info] STTP_NATIVE *not* defined, *not* including native in the aggregate projects") | ||
rawAllAggregates.filterNot(_.toString.contains("Native")) | ||
} | ||
if (sys.env.isDefinedAt("JDK_LOOM")) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hm maybe this should be called |
||
println("[info] JDK_LOOM defined, including only loom-based projects") | ||
filteredByNative.filter(p => (p.toString.contains("Loom") || p.toString.contains("Nima"))) | ||
} else { | ||
println("[info] JDK_LOOM *not* defined, *not* including loom-based-projects") | ||
filteredByNative.filterNot(p => (p.toString.contains("Loom") || p.toString.contains("Nima"))) | ||
} | ||
|
||
} | ||
|
||
// separating testing into different Scala versions so that it's not all done at once, as it causes memory problems on CI | ||
|
@@ -1443,6 +1453,17 @@ lazy val nettyServer: ProjectMatrix = (projectMatrix in file("server/netty-serve | |
.jvmPlatform(scalaVersions = scala2And3Versions) | ||
.dependsOn(serverCore, serverTests % Test) | ||
|
||
lazy val nettyServerLoom: ProjectMatrix = | ||
ProjectMatrix("nettyServerLoom", file(s"server/netty-server/loom")) | ||
.settings(commonJvmSettings) | ||
.settings( | ||
name := "tapir-netty-server-loom", | ||
// needed because of https://github.com/coursier/coursier/issues/2016 | ||
useCoursier := false | ||
) | ||
.jvmPlatform(scalaVersions = scala2_13And3Versions) | ||
.dependsOn(nettyServer, serverTests % Test) | ||
|
||
lazy val nettyServerCats: ProjectMatrix = nettyServerProject("cats", catsEffect) | ||
.settings( | ||
libraryDependencies ++= Seq( | ||
|
@@ -1471,6 +1492,18 @@ def nettyServerProject(proj: String, dependency: ProjectMatrix): ProjectMatrix = | |
.jvmPlatform(scalaVersions = scala2And3Versions) | ||
.dependsOn(nettyServer, dependency, serverTests % Test) | ||
|
||
lazy val nimaServer: ProjectMatrix = (projectMatrix in file("server/nima-server")) | ||
.settings(commonJvmSettings) | ||
.settings( | ||
name := "tapir-nima-server", | ||
libraryDependencies ++= Seq( | ||
"io.helidon.webserver" % "helidon-webserver" % Versions.helidon, | ||
"io.helidon.logging" % "helidon-logging-slf4j" % Versions.helidon | ||
) ++ loggerDependencies | ||
) | ||
.jvmPlatform(scalaVersions = scala2_13And3Versions) | ||
.dependsOn(serverCore, serverTests % Test) | ||
|
||
lazy val vertxServer: ProjectMatrix = (projectMatrix in file("server/vertx-server")) | ||
.settings(commonJvmSettings) | ||
.settings( | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,44 @@ | ||
# Running as a Helidon Níma server | ||
|
||
```eval_rst | ||
.. note:: | ||
Helidon Níma requires JDK supporting Project Loom threading (JDK21 or newer). | ||
``` | ||
|
||
To expose an endpoint as a [Helidon Níma](https://helidon.io/nima) server, first add the following | ||
dependency: | ||
|
||
```scala | ||
"com.softwaremill.sttp.tapir" %% "tapir-nima-server" % "@VERSION@" | ||
``` | ||
|
||
Loom-managed concurrency uses direct style instead of effect wrappers like `Future[T]` or `IO[T]`. Because of this, | ||
Tapir endpoints defined for Nima server use `Id[T]`, which provides compatibility, while effectively means just `T`. | ||
|
||
Such endpoints are then processed through `NimaServerInterpreter` in order to obtain an `io.helidon.webserver.http.Handler`: | ||
|
||
```scala | ||
import io.helidon.webserver.WebServer | ||
import sttp.tapir._ | ||
import sttp.tapir.server.nima.{Id, NimaServerInterpreter} | ||
|
||
val helloEndpoint = endpoint.get | ||
.in("hello") | ||
.out(stringBody) | ||
.serverLogicSuccess[Id] { _ => | ||
Thread.sleep(1000) | ||
"hello, world!" | ||
} | ||
|
||
val handler = NimaServerInterpreter().toHandler(List(helloEndpoint)) | ||
|
||
WebServer | ||
.builder() | ||
.routing { builder => | ||
builder.any(handler) | ||
() | ||
} | ||
.port(8080) | ||
.build() | ||
.start() | ||
``` |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -24,7 +24,8 @@ The modules are categorised using the following levels: | |
| armeria | stabilising | | ||
| finatra | stabilising | | ||
| http4s | stabilising | | ||
| netty | experimental | | ||
| netty | stabilising | | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
| nima | experimental | | ||
| pekko-http| stabilising | | ||
| play | stabilising | | ||
| vertx | stabilising | | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so ... we are trying to only include specific axes? maybe we can either (1) use include instead of exlucde, if possible; (2) if not, at least add a comment, what are the desired combinations that should be included
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the hint, somehow I wasn't aware that there's
include
. Turns out it can be combined withexclude
, because exclusions are resolved first, so our rules will become:Looks clearer to me.