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

Etherpad-Lite: Support docker-compose, fix JSON #19

Closed
wants to merge 1 commit into from

Conversation

pawohl
Copy link

@pawohl pawohl commented Feb 23, 2017

  • Support for docker-compose added to Etherpad-Lite with pre-configured database
  • JSON creation fixed: If ETHERPAD_ADMIN_PASSWORD was not set, a trailing comma after the dbSettings was left. (1) Double quotation marks need escape. (2) settings.json is now created by node-js as ECMA-script supports JSON formatting natively.
  • Customizable database port through ETHERPAD_DB_PORT.

@pawohl
Copy link
Author

pawohl commented Feb 23, 2017

Btw, what is the benefit of removing the apt source lists with rm -r /var/lib/apt/lists/*?

From what I understand, the image isn't getting substantially smaller because the files will be kept in the layers below.

- Support docker-compose added to Etherpad-Lite with
  pre-configured database
- JSON creation fixed: If ETHERPAD_ADMIN_PASSWORD was
  not set, a trailing comma after the dbSettings was
  left. (1) Double quotation marks need escape. (2)
  settings.json is now created by node-js as ECMA-
  script supports JSON formatting natively.
- Customizable database port through
  `ETHERPAD_DB_PORT`.
- BEHAVIOUR CHANGE: Wait for mysql to become ready
  at specified host and port. This allows starting
  the containers with a single `docker-compose up`.
  Etherpad container won't exit if database connec-
  tion can't be established.
@pawohl
Copy link
Author

pawohl commented Feb 24, 2017

I think a fork is better suited. Merge request has issues. Thanks for your work!

@tvelocity
Copy link
Owner

Hello,

Thanks for all your comments. I will take a look in your fork, and eventually merge back any improvements.

Cheers.

@tvelocity
Copy link
Owner

Btw, regarding /var/lib/apt/lists/*, the removal is done under the same RUN directive that populates these files. Under tis scenario, the files will not kept in any of the layers (it is one layer per Dockerfile command).

This is standard practice when writing Dockerfiles, even the official images use this (the official Wordpress image, for example https://github.com/docker-library/wordpress/blob/master/Dockerfile-debian.template )

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