Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

introduce depends_on.service to define the target service #404

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ndeloof
Copy link
Collaborator

@ndeloof ndeloof commented Aug 17, 2023

What this PR does / why we need it:
introduce depends_on.service to define target service. Defaults to mapping key, but could be overridden, typically using a variable.

Which issue(s) this PR fixes:
see docker/compose#10855

Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
@laurazard
Copy link
Member

Is this still relevant? If so, can we maybe think about other names for this property other than service? Maybe target?

@ndeloof
Copy link
Collaborator Author

ndeloof commented Oct 17, 2023

I don't feel depends_on.target: XX to be more readable than depends_on.service: XX

Comment on lines +402 to +403
database:
service: db
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think only having this example might be a bit confusing for new users who don't know about this feature. I'd leave this example as is, and add a specific example about the service key under it's explanation

@@ -380,6 +380,7 @@ expressed in the short form.
to successful completion before starting a dependent service.
- `required`: When set to `false` Compose only warns you when the dependency service isn't started or available. If it's not defined
the default value of `required` is `true`.
- `service`: name of the target service (defaults to mapping key). Value can be set by a variable.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(I'd add the example using this key under this line)

@laurazard
Copy link
Member

Yeah I guess you're right 😅. I left a couple comments just about examples, but otherwise LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants