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

rework docker compose script to allow future extensibility #1002

Merged
merged 4 commits into from
Nov 22, 2023

Conversation

piotrkwiecinski
Copy link
Contributor

This is the first step to allow more flexibility.

For example instead of having blackfire commented out in main compose.yaml.
It could be moved to a separate file and included in the list when feature toggle is enabled.

Something like:

if [ ${BLACKFIRE_ENABLED} -eq 1 ]; then
  COMPOSE_FILES_LIST+=("compose.blackfire.yaml")
fi

It would allow adding features like #998 the same way.

Copy link

what-the-diff bot commented Nov 10, 2023

PR Summary

  • Enhanced Support for Multiple Compose Files
    The new changes allow for the loading of multiple Docker compose files. These files are stored in an array called COMPOSE_FILES. This provides a more flexible and efficient way to manage multiple Docker compose files in the system.

  • Optional Exclusion of the Development Configuration File
    The system has been improved to handle the --no-dev argument. Users can use this argument to exclude the development configuration file compose.dev.yaml when they do not want to use it.

  • Enhanced Command Execution
    A new method for executing Docker compose commands has been put in place. It constructs command-line parameters dynamically, accommodating either single or multiple compose files. This increases efficiency and reliability when executing various commands.

@YPyltiai
Copy link
Contributor

@piotrkwiecinski part of the reason I added Cloudflare to same file is to have it in same network as the rest of app. If you have it separately - I guess we'd manually need to set networks too. But good idea anyways!

@piotrkwiecinski
Copy link
Contributor Author

It's going to be my next PR. I want to link services using network, so these changes are easier.

Copy link
Owner

@markshust markshust left a comment

Choose a reason for hiding this comment

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

Just need a few comments here to explain what the code is doing. Thanks!

compose/bin/docker-compose Show resolved Hide resolved
@markshust
Copy link
Owner

Thanks so much, great update!

@markshust markshust merged commit a5a73d5 into markshust:master Nov 22, 2023
1 check passed
Copy link
Owner

@markshust markshust left a comment

Choose a reason for hiding this comment

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

This is all set and should be closed out 👍

@what-the-diff what-the-diff bot mentioned this pull request Feb 25, 2024
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