Conversation
| case _ @("scala/annotation/nowarn.class" | "scala/annotation/nowarn$.class") => MergeStrategy.first | ||
| case x => (assembly / assemblyMergeStrategy).value(x) | ||
| }, | ||
| Test / test := (Test / test) |
There was a problem hiding this comment.
if you'd like to run it using 2.13 only, I think you could do sth like:
Test / test := if (scala.version == scala2_13) {...} else ()
There was a problem hiding this comment.
did you manage to run the tests on 2.13 only or is this solved some other way?
|
|
||
| lazy val sam = Process("sam local start-api --warm-containers EAGER").run() | ||
|
|
||
| lazy val awsLambdaTests: ProjectMatrix = (projectMatrix in file("serverless/aws/lambda-tests")) |
There was a problem hiding this comment.
I'd add a comment detailing what the test is doing and why is it done this way
| ``` | ||
|
|
||
| The second one is `AwsSamInterpreter` which interprets Tapir `Endpoints` into AWS SAM configuration file. | ||
| It should be used to configure your API Gateway. |
There was a problem hiding this comment.
we need terraform section in the docs :)
There was a problem hiding this comment.
added, also instructions on how to run examples
| /** Before running the actual example we need to interpret our api as Terraform resources */ | ||
| object TerraformConfigExample extends App { | ||
|
|
||
| val jarPath = Paths.get("serverless/aws/examples/target/jvm-2.13/tapir-aws-examples.jar").toAbsolutePath.toString |
There was a problem hiding this comment.
can we maybe pass in the "project home" directory as an env variable?
There was a problem hiding this comment.
I can access System.getProperty("user.dir") which gives me the path to tapir root directory, that's not what we need. Or do you thought about some other system property / env variable here?
There was a problem hiding this comment.
Well I guess this depends a lot on how you run this example. Does this work from intellij or from sbt console?
There was a problem hiding this comment.
Both, Paths.get(...) starts from project root directory so it works regardless of where the project is located
|
|
||
| val interpreter = new AkkaHttpTestServerInterpreter()(actorSystem) | ||
| val createServerTest = new CreateServerTest(interpreter) | ||
| val createServerTest = new DefaultCreateServerTest(backend, interpreter).asInstanceOf[DefaultCreateServerTest[Future, AkkaStreams with WebSockets, Route, AkkaResponseBody]] |
There was a problem hiding this comment.
I suspect the .asInstanceOf is a type inference issue - maybe it would help if we explicitly added type parameters?
There was a problem hiding this comment.
The backend has FS2Streams as capability and here we require AkkaStreams for ServerWebSocketTests and ServerStreamingTests, so .asInstanceOf was the only option
There was a problem hiding this comment.
and this works? if the streams are incompatible, it cannot possibly work
There was a problem hiding this comment.
ah I think I see where the problem is. It's wrong to require that the R of the backend is the same as of the interpreter. The backend's streaming can always be Fs2? as that's what we are using in tests.
There was a problem hiding this comment.
why did we need to parametrise with the backend in the first place? I know there was a reason but forgot what it was ;)
There was a problem hiding this comment.
To be able to run tests like ServerBasicTests with stubbed backend (AwsLambdaCreateServerStubTest). While doing that each test needs a new instance of StubBackend. And since other tests does not require new backend instance I parameterized the DefaultCreateTestServer with it.
| :caption: Server interpreters | ||
|
|
||
| server/akkahttp | ||
| server/aws |
There was a problem hiding this comment.
it's a bit ironic that's a "serverless" interpreter is listed under "server interpreters", but I guess that's the proper place ;)
|
|
||
| private def safeRead(byteBuffer: ByteBuffer): Array[Byte] = { | ||
| if (byteBuffer.hasArray) { | ||
| byteBuffer.array() |
There was a problem hiding this comment.
I think the array can be larger than the actual data. See: softwaremill/sttp@d1e3ec3
We'd have to check the array size, the buffer's limit and if they don't match, create a copy
Follow-up task (not in this PR): check other interpreters if they don't have the same bug
|
|
||
| import scala.reflect.ClassTag | ||
|
|
||
| trait AwsServerInterpreter { |
There was a problem hiding this comment.
maybe let's call it AwsCatsEffectServerInterpreter - as that's what it is
There was a problem hiding this comment.
we'll prepare the naming schemes for Future and ZIO interpreters :)
| libraryDependencies ++= loggerDependencies, | ||
| libraryDependencies ++= Seq( | ||
| "com.softwaremill.sttp.client3" %% "http4s-ce2-backend" % Versions.sttp, | ||
| "org.http4s" %% "http4s-blaze-client" % Versions.http4s |
There was a problem hiding this comment.
probably a follow-up task: I think we are always using this with Java 11+? If so, we might be able to use the HttpClient backend, which would reduce the dependencies of this project (to just cats-effect and regular java)
| implicit val monad: MonadError[F] = new CatsMonadError[F] | ||
|
|
||
| val runtimeApiInvocationUri = uri"http://${sys.env("AWS_LAMBDA_RUNTIME_API")}/2018-06-01/runtime/invocation" | ||
| val nextEventRequest = basicRequest.get(uri"$runtimeApiInvocationUri/next").response(asStringAlways).readTimeout(0.seconds) |
There was a problem hiding this comment.
why do we have 0 sec timeout here?
There was a problem hiding this comment.
It is handled this way in Scalambda, they say that:
Make request (without a timeout as prescribed by the AWS Custom Lambda Runtime documentation).
This is due to the possibility of the runtime being frozen between lambda function invocations.
Continuously polling also seems to cause issues with AWS's internal lambda state management service,
so this definitely seems to be the best way of fetching the event.
I've searched in lambda custom runtimes documentation but found no explicit mention about timeouts there.
There was a problem hiding this comment.
ah, so 0 timeout is mean to disable it. Ok, let's add a comment maybe where this comes from?
| _ <- sendResult(event, result) | ||
| } yield () | ||
|
|
||
| while (true) ConcurrentEffect[F].toIO(loop).unsafeRunSync() |
There was a problem hiding this comment.
won't this blow up when there's any exception? we log the error and propagate it always, so shouldn't have some catch-all handler in the end?
another thing: can't we do sth like loop.forever.unsafeRunSync() instead of the while?
There was a problem hiding this comment.
right, final handle error is missing, there is foreverM in cats
|
|
||
| implicit val options: AwsServerOptions[IO] = AwsServerOptions.customInterceptors(encodeResponseBody = false) | ||
|
|
||
| allEndpoints.foreach(e => println(e.endpoint.showDetail)) |
| .value, | ||
| Test / testOptions ++= { | ||
| val log = sLog.value | ||
| lazy val sam = Process("sam local start-api --warm-containers EAGER").run() |
There was a problem hiding this comment.
we might add a comment that this uses the template.yaml which is generated by the application above - if I understand correctly :)
| import sttp.tapir.server.ServerEndpoint | ||
| import sttp.tapir.server.tests.backendResource | ||
|
|
||
| class AwsLambdaSamLocalHttpTest extends AnyFunSuite { |
There was a problem hiding this comment.
again, here I would add a comment that this assumes that a sam-local process is started (this setup is non-obvious so needs documenting)
| import scala.collection.JavaConverters._ | ||
|
|
||
| // modified stringNode to handle !Ref tags | ||
| final case class Printer( |
There was a problem hiding this comment.
another follow-up task: add a PR to https://github.com/circe/circe-yaml which would propagate this change.
Didn't we have another modification elsewhere with a "fixed" isBad method?
There was a problem hiding this comment.
We thought about fixing isBad to handle string literals with some special characters automatically but we finally settled for configuring that in open api toYaml methods
|
|
||
| case class AwsTerraformApiGateway(routes: Seq[AwsApiGatewayRoute]) { | ||
| def toJson()(implicit options: AwsTerraformOptions): String = { | ||
| val gateway = this |
There was a problem hiding this comment.
right it's not actually required, I thought it was since IDE is showing an error in this line when I use this.asJson
| sendError(event, e) | ||
| } | ||
|
|
||
| (for { |
There was a problem hiding this comment.
maybe it should be named AwsLambdaRuntimeLogic instead of loop
2 participants
No description provided.