-
-
Notifications
You must be signed in to change notification settings - Fork 166
Fix docker compose #1618
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
Fix docker compose #1618
Conversation
drgrice1
left a comment
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 looks good.
taniwallach
left a comment
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.
The corrections all seem correct, and I was able to build using the revised DockerfileStage1 and to get the container to start (and course pages to work) using the corrected docker-config/docker-entrypoint.sh.
I'm not sure why there is an active ADD_APT_PACKAGES line in docker-compose.yml. That seems to have crept in as part of #1435 but using that slows down the container startup time significantly. I would tend to recommend that it be commented out by default. People who need such packages might want to modify their DockerfileStage2 to add extra packages instead. Fixing this can wait.
|
@taniwallach: I am not sure that it crept in. You and @mgage had a discussion about that. See #1435 (comment). |
Sorry - you are correct. I did not review the history / conversation carefully, just tried to determine in which PR where it came in. I even was the one to merge that PR, as I guess @mgage convinced me that the approach was reasonable, and users could comment it out if the delay was annoying. As someone who starts/stops Docker containers frequently, both when doing development and for maintenance on productions servers running WW inside Docker - I think keeping container startup time down is quite important. A better long term solution might be to add conditional code so that extra packages like Let's get these urgent fixes merged, and decide later about how to better add packages helpful for developers and not mission critical on development servers. |
There seem to be two issues that come up with the current
docker-composeroutine, i.e. when creating the container via:docker-entrypoint.shdocker-composecomplains aboutEmail::Stuffermissing