introduce restart for depends_on#305
Conversation
milas
left a comment
There was a problem hiding this comment.
LGTM, I nitpicked the wording but no concerns on the actual spec/schema
spec.md
Outdated
| The long form syntax enables the configuration of additional fields that can't be | ||
| expressed in the short form. | ||
|
|
||
| - `restart`: when `true` a Compose implementation MUST restart service after dependent service has been restarted. |
There was a problem hiding this comment.
| - `restart`: when `true` a Compose implementation MUST restart service after dependent service has been restarted. | |
| - `restart`: when `true` a Compose implementation MUST restart this service after updates to the dependency service. |
(English is horrible here, there's no non-confusing way to talk about dependencies. 😡) Looking at this section, though, we refer to the "parent" service as "dependency service" and the "child" service as dependent, so trying to be consistent
Also, I'm not crazy about my change from "has been restarted" to "after updates" - we probably need another sentence of clarification here to be explicit that the restart will only happen as part of a Compose operation, e.g. if parent dies and gets auto-restarted by Docker Engine, nobody is going to restart child in that case. That is, this is a "deployment" (up) level thing, there's no background daemon monitoring
There was a problem hiding this comment.
We could make it more explicit as
" a Compose implementation MUST restart this service after it updated the dependency service. " so that users don't expect some magic to happen :)
0ddcd39 to
c8cb728
Compare
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
c8cb728 to
d377dcf
Compare
What this PR does / why we need it:
introduce additional
restartattribute ondepends_on.This let user explicitly opt-in for service to be restarted after a dependency has been restarted.