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

Refactoring Docker to utillise latest features of compose syntax vers… #20

Merged
merged 1 commit into from
Aug 18, 2016

Conversation

rawkode
Copy link

@rawkode rawkode commented Aug 16, 2016

Q A
Bug fix? no
New feature? no
BC breaks? no
Related tickets fixes #X, partially #Y, mentioned in #Z
License MIT

…ion 2

@rawkode
Copy link
Author

rawkode commented Aug 16, 2016

This shouldn't be merged yet. The only issue I can see is that sylius:install is interactive. How can this be automated? :)

Sorry, I don't know Sylius that well

@pamil
Copy link
Owner

pamil commented Aug 17, 2016

Nice, thanks! Is it a common practice to set up the application at the entrypoint? We've got bunch of different environments (prod, dev, test, test_cached) and we don't know which one is needed at the moment.

@rawkode
Copy link
Author

rawkode commented Aug 17, 2016

Ideally you want people to be able to do docker-compose up and your application is running. The entrypoint is only defined in docker-compose.yml, which 99.99% of the time is a development environment.

For test runs, I'd suggest adding to the docs, docker-compose run -u www-data sylius /var/www/etc/docker/php/entrypoint.sh test or using ENV, docker-compose run -e SYMFONY__ENVIRONMET=test -u www-data sylius, toggles to specify the environment - both are good solutions

Is there a way to make sylius:install work without input? If not, then we can provide helper scripts instead; it's just not a fluid :)

@pamil
Copy link
Owner

pamil commented Aug 17, 2016

I guess that providing dev environment by default would be all right for now, we can allow for other ones in the future.

sylius:install can be replaced with some another commands, the same way as we set up the application on Travis:

app/console doctrine:database:create
app/console cache:warmup
app/console doctrine:migrations:migrate --no-interaction
app/console doctrine:phpcr:repository:init
app/console assets:install

npm run gulp

The another issue is that we are using node.js for our frontend, I guess we can include it in our sylius image.

@@ -3,14 +3,15 @@

imports:
- { resource: "config.yml" }
- { resource: "parameters_dev.yml" }
Copy link
Owner

Choose a reason for hiding this comment

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

Hmm, it would be nice not to have it, but we will have to provide the correct hostname anyway :/

Copy link
Owner

@pamil pamil Aug 17, 2016

Choose a reason for hiding this comment

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

I might have an idea for that - creating etc/docker/config/parameters.yml with the following content:

parameters:
    database_host: database
    # any other settings specific for Docker

and then replacing the app/config/parameters.yml with it before running the entrypoint.

The only drawback I can think of it's that the original file gets overwritten - can we avoid it by just mounting that file instead of copying it?

Choose a reason for hiding this comment

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

Can't we just change a data of parameters.yml between composer install and sylius:install?

Copy link
Owner

Choose a reason for hiding this comment

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

That will change that file on the host machine too.

@pamil
Copy link
Owner

pamil commented Aug 17, 2016

Thank you very much! I left some questions in between the code as it's the best chance for me to learn Docker in practice. There are also a few suggestions for tweaks to integrate it with Sylius even better.

@lchrusciel
Copy link

@rawkode sylius:install can be run in no interaction mode. You have to add a --no-interaction flag to the command. AFAIR it would set up a default dev environment, but will not load fixtures, so I would also recommend to add sylius:fixtures:load

@lchrusciel
Copy link

Of course an environment flag can be attach also to no interaction mode.

@pamil pamil merged commit 668a59b into pamil:docker Aug 18, 2016
@pamil
Copy link
Owner

pamil commented Aug 18, 2016

Thanks David! I'll make some Sylius related customizations and see how it looks in practice.

@rawkode
Copy link
Author

rawkode commented Aug 18, 2016

OK, @pamil - once you've done that, I'll make some further tweaks that I hadn't had time to complete. Good work 👍

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