-
Notifications
You must be signed in to change notification settings - Fork 4
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
Add ability to modify docker-compose configuration #722
base: master
Are you sure you want to change the base?
Conversation
This adds the ability for modules to extend the docker-compose.yml configuration with sidecar containers (or override existing configuration).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple of suggestions.
Also, the lint issues will need fixes (mostly missing docs).
docs/extra-containers.md
Outdated
|
||
## How Local Server works | ||
|
||
Local Server internally uses Docker Compose to create and manage containers for the Altis environment. When a user runs the `composer server start` command, Local Server dynamically provisions a `docker-compose.yml` file based on the user's preferences. (This file is only regenerated when starting Local Server to avoid conflicts or surprising behaviour for users.) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Local Server internally uses Docker Compose to create and manage containers for the Altis environment. When a user runs the `composer server start` command, Local Server dynamically provisions a `docker-compose.yml` file based on the user's preferences. (This file is only regenerated when starting Local Server to avoid conflicts or surprising behaviour for users.) | |
Local Server internally uses Docker Compose to create and manage containers for the Altis environment. When a user runs the `composer server start` command, Local Server dynamically provisions a `docker-compose.yml` file based on the user's preferences. (This file is only regenerated when starting Local Server to avoid conflicts or surprising behaviour for users.) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you also wrap lines at 132 chars please? As per our MD linting standards.
inc/composer/class-command.php
Outdated
$config = []; | ||
foreach ( $packages as $name => $package ) { | ||
$extra = $package->getExtra(); | ||
if ( ! isset( $extra['altis'] ) || ! isset( $extra['altis']['local-server'] ) ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would be clearer as a positive test:
if ( ! isset( $extra['altis'] ) || ! isset( $extra['altis']['local-server'] ) ) { | |
if ( isset( $extra['altis'] ) && isset( $extra['altis']['local-server'] ) ) { | |
$config[ $name ] = $extra['altis']['local-server']; | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd agree if it were a straight if
/else
, but this is more of a return-early pattern. Not strongly opinionated so if you feel it's much more readable that way I'll update
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe empty
would read better. if (empty( $extra['altis']) || (empty($extra['altis']['local-server']))
I like the idea of this. |
@rmccue Just noting that we'd love to have the ability to do modify the docker compose from our own project! Our immediate use case for this would be to prefix all of the image names in the generated docker-compose file with our internal docker hub proxy URL so they get pulled from there instead of directly from docker hub which will prevent us from running into docker hub API limits in CI. It looks like we'd be able to achieve this quite easily with the way this feature has been designed 😃 |
Co-authored-by: Konstantin Kovshenin <[email protected]>
I did test this locally with the ES example. It highlighted a weird error that I think it is a dependency issue, though it doesn't make sense. $ composer server start
Starting...
PHP Fatal error: Declaration of Symfony\Component\Console\Input\ArrayInput::hasParameterOption(array|string $values, bool $onlyParams = false): bool must be compatible with Symfony\Component\Console\Input\InputInterface::hasParameterOption($values, bool $onlyParams = false) in /Users/mikelittle/sandbox/02-altis/product-dev/vendor/symfony/console/Input/ArrayInput.php on line 50
Fatal error: Declaration of Symfony\Component\Console\Input\ArrayInput::hasParameterOption(array|string $values, bool $onlyParams = false): bool must be compatible with Symfony\Component\Console\Input\InputInterface::hasParameterOption($values, bool $onlyParams = false) in /Users/mikelittle/sandbox/02-altis/product-dev/vendor/symfony/console/Input/ArrayInput.php on line 50 Grep-ing for the function gets me three hits—all correct: $ grep -r 'hasParameterOption'
... skipping calls
vendor/symfony/console/Input/ArrayInput.php: public function hasParameterOption(string|array $values, bool $onlyParams = false): bool
vendor/symfony/console/Input/ArgvInput.php: public function hasParameterOption(string|array $values, bool $onlyParams = false): bool
vendor/symfony/console/Input/InputInterface.php: public function hasParameterOption(string|array $values, bool $onlyParams = false): bool; So far, I made it work by specifying However, this issue apart, the code did work. I could enable/disable the Elastic Search container from the Enhanced Search module. |
Adds the ability for any Composer package (including the root/project itself) to modify the docker-compose configuration, making it possible for modules to add containers or alter the existing configuration.
This can be used for better modularity and conditional behaviour, such as for optional features like Elasticsearch or Node.js. It also provides the ability for users to customise Docker behaviour fairly directly in their project.
My plan is to transition the ES + Kibana containers to the Enhanced Search package, and then eventually someday remove it from the default bundle, allowing a clearer opt-in for customers who have actually bought it. PR for that incoming soon.