grpc / grpc-swift

The Swift language implementation of gRPC.
Apache License 2.0
2.01k stars 416 forks source link

In v2, the generated gRPC server should conform to swift-service-lifecycle Server #1710

Closed Jerry-Carter closed 10 months ago

Jerry-Carter commented 10 months ago

I've seen some discussion around names and the Swift Service Lifecycle but no clear statement on the relationship between gRPC Swift and it. I propose that the server code generated optionally conform to the swift-service-lifecycle Server class.

Motivation: The Swift Service Lifecycle ServiceGroup provides a simple method for launching services and supporting graceful termination in response to signals such as TERM and INT. Making gRPC Swift servers conform simplifies integration and management.

glbrntt commented 10 months ago

We will make it possible to integrate with swift-service-lifecycle. Whether that's out-of-the-box or users have to write small shims is yet to be seen though.

Jerry-Carter commented 10 months ago

Firstly, no objection to marking this closed. Secondly, my preference, to the extent that it matters, is the former.

You have almost certainly thought through this weeks or months ago, so apologies if I state many obvious things. The gRPC generated code should not, by default, conform to the swift-service-lifecycle Service interface as that would impose a dependency on every user. There is an obvious question as to how much traction ServiceLifestyle has within SSWG and in the community of developers. As an external observer, the answer at present appears to be 'moderate' and 'minimal' respectively with the expectation that critical mass within SSWG will drive adoption externally for the future. If someone performs an integration between gRPC-swift and swift-service-lifecycle to verify that integration is possible, I prefer that this be added to the generated code or at the very least published for reference. My selfish preference is that integration be incorporated into the generated code as that means less code for me or other members of the team to maintain. That and the code from the gRPC-swift team will likely have fewer bugs. 😜

FranzBusch commented 10 months ago

FWIW, we are already seeing adoption of swift-service-lifecycle in various repositories that are new such as swift-kafka-client or swift-memcache-gsoc. For repositories that are older and existed before Swift Concurrency came around the adoption of swift-service-lifecycle is way more difficult and often not really doable. The intention with the new version of swift-service-lifecycle was to cater for a world where only structured Concurrency existed.

For grpc-swift the generated could should probably never add a ServiceLifecycle.Service conformance. The only place where I can see such a conformance make sense is on a GRPCServer and potentially GRPClient type. We have to make sure that there is an integration with swift-service-lifecycle somewhere since it will allow us to have a very nice story for graceful shutdown in gRPC-swift and throughout the ecosystem. If that integration is in a core module or behind a feature flag we have to see.

Jerry-Carter commented 10 months ago

Thank you for that background. My only thought was around GRCPServer as our current client code wouldn't benefit from ServiceLifecycle.Service. I agree that GRPClient might benefit in cases where one server depends on a second. Between you (Franz) and George, I trust your decisions in this matter.