Reviews are an important part of the Swift Server Work Group incubation process. All review feedback should be either on this forum thread or, if you would like to keep your feedback private, directly to the review manager (via email or direct message in the Swift forums).
What goes into a review of a proposal?
The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, become listed on the Swift Server Ecosystem index page .
When reviewing a proposal, here are some questions to consider:
What is your evaluation of the proposal and its APIs/implementation?
Is the problem being addressed relevant for the wider Swift on Server ecosystem?
Does this proposal fit well with the feel and direction of the Swift Server ecosystem?
If you have used other libraries in Swift or other languages, how do you feel that this proposal compares to those?
How much effort did you put into your review? A glance, a quick reading, or an in-depth study?
+1 this is great. It generalizes functionality that high-level frameworks like Vapor would otherwise have to repeat and provides the basic building blocks users rolling their own solutions would need.
One question I had when looking at the proposal was around the difference between ServiceLifecycle and ComponentLifecycle. Why do these need to be separate types?
struct SubSystem {
let lifecycle = ServiceLifecycle(label: "SubSystem")
let subsystem: SubSubSystem
init() {
self.subsystem = SubSubSystem()
self.lifecycle.register(self.subsystem.lifecycle)
}
struct SubSubSystem {
let lifecycle = ServiceLifecycle(label: "SubSubSystem")
init() {
self.lifecycle.register(...)
}
}
}
let lifecycle = ServiceLifecycle()
let subsystem = SubSystem()
lifecycle.register(subsystem.lifecycle)
lifecycle.start { error in
...
}
lifecycle.wait()
I imagine the way Vapor would adopt this package is by wrapping ServiceLifecycle in its Application type. That would be the most similar to what we are currently doing today.
Now assume that someone wants to add a Vapor Application as a subcomponent in a larger system. Since Vapor's using ServiceLifecycle instead of ComponentLifecycle would this not be possible?
Disclaimer: I don't really care much about the two being two types, but following the current design:
As a framework/library, shouldn't it be a component rather than a service? Only the end user's system is "the service", it often may happen to be just the vapor app, but maybe it's a number of things and the vapor app.
You could also look at Tasks with dependsOn and topological sort of task start order · Issue #49 · swift-server/swift-service-lifecycle · GitHub which IMO is a more powerful / compositional way to express these many systems / components style. It's modeled after what we successfully had in Akka and Akka HTTP etc made good use of it. So rather than users having to register lifecycles in the right order, lifecycle tasks declare if they have dependencies, and the tasks are spawned in the right order.
The current design is brittle, because it depends on users being able to register all the phases in the specific order they want to run, rather than declare them... Examples here Coordinated Shutdown • Akka Documentation
So that might be an alternate or extended consider; perhaps there would be no need for Component Lifecycle then?
It's my understanding that ComponentLifecycle doesn't have the start and wait methods. Vapor's Application type is the top-level thing that a user declares in their main.swift and tests. For example:
// main.swift
let app = try Application(.detect())
defer { app.shutdown() }
app.get("hello") { req in
"world"
}
try app.run()
In other words, Application wraps everything that Vapor does into a single init() and run().
I imagine app.run and app.shutdown would call ServiceLifecycle's startAndWait and shutdown respectively.
However, if you want to plug Vapor into something else, you should be able to do something like:
let lifecycle = ServiceLifecycle()
// vapor
let app = try Application(.detect())
lifecycle.register(app.lifecycle)
// other stuff
try lifecycle.startAndWait()
I guess this could be implemented by Application exposing two properties, like app.lifecycle and app.componentLifecycle or something. But I think it would be easier to use if they were just the same type. Any framework that uses ServiceLifecycleshould be pluggable as a component in a larger lifecycle anyway, right?
ServiceLifecycle is used when you need to set up shutdown signal listener and install the backtraces which you only want to do once per process, while ComponentLifecycle does not set those up. In most cases ComponentLifecycle is not required and its easy enough to compose by having the subsystem expose its start and shutdown APIs (normally just shutdown), but in very larger systems ComponentLifecycle proved as a useful composition block as it allows such systems to more easily reason about the start and shutdown hierarchy.
This is a fair point, but we may need to distinguish between libraries and application frameworks here.
Libraries would normally not need to use the lifecycle library and just expose public start and shutdown APIs (normally just shutdown) e.g. async-http-client, db drivers that use swift-nio EventLoopGroup, etc. If a library does decides to use the lifecycle library for composition reasons than you are 100% right that it should use ComponentLifecycle and not set up shutdown hooks on behalf of the user but I would also argue that in those cases the lifecycle should remain an implementation detail and the library should only expose start and shutdown APIs, not the underlying ComponentLifecycle.
On the other hand, application frameworks like vapor normally offer their users start and stop APIs that "just work" (including shutdown hooks and backtraces) and do not need to be composed further, which is what ServiceLifecycle is designed to solve. One could argue that the composition gives users more control, but I think most web-framework users expect the framework to be the top level entity they interact with.
Obviously this is an opinion and open to discussion.
See above - I believe this analysis is correct and that an application framework like vapor should provide it's top level start/stop functionality via ServiceLifecycle.
Yes, that is the goal. As you note, the tricky part here is that in an application framework like vapor you want to offer two kind of start API (shutdown is the same in both cases):
start that set up the shutdown hook and backtrace (most common)
start that defers setting up the shutdown hook and backtrace to the caller for composition (less common)
To support the second use case, vapor application can either:
expose the "raw" start method that does not go through ServiceLifecycle::start, but rather is the internal method that ServiceLifecycle::start calls
expose lifecycle and its "guts"
The design thinking behind ComponentLifecycle was that it would be an internal composition implementation detail in complex systems and not something that "leaks" via other libraries/frameworks APIs. As such, I tend to think option 1 would be better - just need to find a good names for the two different start APIs.
Using Vapor as a single piece in a larger application:
let lifecycle = ServiceLifecycle()
let app = try Application(.detect())
// app.lifecycle is ComponentLifecycle
lifecycle.register(app.lifecycle)
// other stuff
try lifecycle.startAndWait()
2: To expose a ComponentLifecycle but use a separateServiceLifecycle internally.
I much prefer option #1 since, to me at least, it makes sense that if you have something using a ServiceLifecycle, you should always be able to plug that as one piece into a larger system. It would be unfortunate if framework maintainers had to explicitly allow you both options.
Side note after digging into the code more: Is ServiceLifecycle.startAndWait()not installing signal handlers a bug?
As primarily a library author instead of a framework, I'm trying to figure out what my adoption of this would be.
You mentioned that we should provide just the start/stop hooks, but should we also adopt this API library and conform our types to it, probably as a compatibility target?
Or should it be left as an exercise to higher-level libraries and frameworks, even though it breaks the Swift best practice of not conforming types you don't own to protocols you don't own?
In my opinion, most library authors do not need to depend on the lifecycle library, and should only offer start / shutdown APIs when otherwise necessary. For example, a database-driver or http-client that uses SwiftNIO and creates an instance of EventLoopGroup should expose a shutdown API anyways, and in my opinion that is enough for higher level frameworks / applications to plug that library into their lifecycle hierarchy.
