akka / akka-grpc

A platform to build and run apps that are elastic, agile, and resilient. SDK, libraries, and hosted environments.
https://doc.akka.io/libraries/akka-grpc/current/
Other
431 stars 124 forks source link

Generate invalid code for server side in play-java #523

Closed zhengcan closed 1 year ago

zhengcan commented 5 years ago

Akka gRPC Version: 0.5.0

The PR #502 and #511 introduce some conflict changes between akka-grpc/codegen/src/main/twirl/templates/PlayJavaServer/Router.scala.txt and akka-grpc/runtime/src/main/scala/akka/grpc/javadsl/GrpcExceptionHandler.scala. These conflict changes haven't been fixed in 0.5.0, which make akka-grpc generate invalid code for play-java.

For example, in Router.scala.txt

  public Abstract@{service.name}Router(Materializer mat) {
    this(mat, GrpcExceptionHandler.defaultMapper());
  }

  public Abstract@{service.name}Router(Materializer mat, Function<Throwable, Status> eHandler) {
    this.eHandler = eHandler;
    super(mat, @{service.name}.name);
  }
johanandren commented 5 years ago

Thanks for letting us know, strange that it didn't surface in the integration tests.

bjaglin commented 5 years ago

This is not specific to Java as I ran into the same issue with Play Scala - it looks like Play codegen is not covered by any integration test in this repo actually.

There are tests in play-grpc though, and I was confused to see the play-grpc 0.5.0 green, but I realized versions were not aligned across repos even though the 0.5.0 tags came almost at the same time.

Is the plan to move the Play twirl templates over there somehow?

johanandren commented 5 years ago

Yeah, there is some work in progress on that. @dwijnand I think you know the status best, maybe you could fill in something here?

dwijnand commented 5 years ago

It wasn't in progress originally as I thought we were trying to move away from having "Play-specific" things.

However more recently we've decided to unburden the akka-grpc repo of Play things.

Unfortunately there's some very long lived PRs that are blocking that progress, see https://github.com/akka/akka-grpc/issues/517.

patriknw commented 1 year ago

@johanandren Do you understand the status of this issue? Can we close it here?

johanandren commented 1 year ago

Play gRPC is no longer a part of this repo, so I think we can close.