Skip to content
This repository was archived by the owner on Apr 17, 2024. It is now read-only.

Conversation

@wilroboly
Copy link

I had a really good time working with the build, but I can into some challenges with my local setup. I found I needed to manage some ports and versions of things here and there. I offer the following changes to improve the build process and the instructions for putting this awesome project together. I hope you find them as useful as I did.

MYSQL_ALLOW_EMPTY_PASSWORD=yes
MYSQL_ROOT_PASSWORD=root

HOSTNAME=contenta.local
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 that for decoupled development, it's maybe good having localhost as the host so things like service workers and so on don't get blocked?

Copy link
Author

Choose a reason for hiding this comment

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

That's a really good idea, as long as its on a port I guess. We really don't want to clobber other localhost apps that might be running on the host.

MYSQL_ROOT_PASSWORD=root

HOSTNAME=contenta.local
HOSTIP=192.168.1.1
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 the final goal would be to have a network set up instead of fixing an IP?

Copy link
Author

Choose a reason for hiding this comment

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

Ya, that would likely be a better idea. I was just putting this in as I didn't want it in the docker-compose.yml. Again, the more generic, the better, I suppose.


HOST_MYSQL_PORT=3336
HOST_PHP_PORT=9009
HOST_HTTP_PORT=8888
Copy link
Member

Choose a reason for hiding this comment

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

Ports maybe good to have by default but why don't use the default ports?

I'm guessing adding a .env.defaults with all this would be better instead and add .env to the gitignore

Copy link
Author

Choose a reason for hiding this comment

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

Ya, the defaults would be a good idea to use as a common base, I put those in as I wanted to show an example of slightly modified ports. But I'm fine with whatever.

chmod +x /usr/local/bin/docker-entrypoint && \
chmod +x /usr/local/bin/init-drupal

ARG CMS_VERSION=8.x-1.x
Copy link
Member

Choose a reason for hiding this comment

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

CONTENTA_VERSION?

Copy link
Author

Choose a reason for hiding this comment

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

Ya, that's the contenta_version. Obviously, the base here. I added the CMS_VERSION as this is how it was identified somewhere in the contenta build. So, just took a cue off them.

ep /etc/php-fpm.conf
ep /etc/php-fpm.d/*

[ ! -e /run/php-fpm ] && mkdir -p /run/php-fpm
Copy link
Member

Choose a reason for hiding this comment

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

Why is this needed?

Copy link
Author

Choose a reason for hiding this comment

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

I noticed in one of the forks, this was a problem for some, where the fpm workers weren't starting due to a missing path. I put it in just as a precaution. I've also noticed this setup in docker4drupal, so it must be a common fix.

Copy link

Choose a reason for hiding this comment

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

this solves the issue i run into, where its missing that folder and errors out thus php container never runs, seems like this issue was reported here: #17

Copy link
Member

Choose a reason for hiding this comment

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

👍

@pcambra
Copy link
Member

pcambra commented Apr 19, 2018

I think maybe you could break this PR in smaller ones and we can get in some of the changes you're proposing?

Many thanks for working on this! 💪

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants