docker: make settings fully configurable via env vars by sapkra · Pull Request #3873 · ether/etherpad

sapkra · 2020-04-16T21:27:53Z

Refactored docs for env vars using tables
Added new env vars for nearly all configuration options
Added possibility to set env setting to null using env vars

Please note: I was not able to test everything. Theoretically everything should work.

muxator · 2020-04-16T21:35:19Z

Wow, nice work, thanks!

In the weekend I'll test it and merge ASAP.

flecno · 2020-04-17T23:13:01Z

Yeah! Very nice! Thank you so much 🎉 🙌

flecno · 2020-04-17T23:14:24Z

What is wrong with the CI tests?

muxator · 2020-04-19T01:10:26Z

Now that #3744 is merged I'll have to do a small rework.

sapkra · 2020-04-19T17:25:36Z

@muxator I've updated the PR to include the skin variants.

Now every setting in the official Etherpad container will be configurable via environment variables.

muxator · 2020-04-19T20:55:03Z

@muxator I've updated the PR to include the skin variants.

Thanks, it's not common seeing such attention. Really appreciated.

muxator · 2020-04-19T21:29:35Z

Added possibility to set env setting to null using env vars

I am going to split this in a dedicated commit immediately before your change.

muxator · 2020-04-19T21:40:41Z

+    if (stringValue === "null") {
+      /*
+       * There is already a way of setting a default value to null, and it is
+       * by using the syntax ${VAR_NAME}. This allows to use ${VAR_NAME:null} as
+       * well.
+       */
+      return null;
+    }


I have split this modification in a dedicated commit (55dd44c) and tried to document it.

However, I am not sure if it's better to include this or not, because it introduces an edge case (the string "null" would become special, and a crazy admin would no longer be able to call his user "null").

In theory, there is already complete syntactic support for "null" values without introducing special strings. See for example:

https://github.com/ether/etherpad-lite/blob/70990afd6680045b9887559b18e8a380aabc814c/settings.json.docker#L59-L66

On the other side, I understand that ${VAR_NAME:null} is more explicit than ${VAR_NAME}.

My questions for you are:

did you introduce this snippet because the documentation was not evident enough? In this case, I would prefer to drop this change & improve the documentation;

or did you do it because you were aware of the existing support but found it uncomfortable? In this case, let's discuss. I do not like the edge case, but user ergonomics is important.

muxator

Super useful PR. I have a doubt about introducing coercition of ${VAR_NAME:null}, since we already support it via ${VAR_NAME}.

I'd prefer to improve the documentation without introducing a potential brittleness. But I understand the proposed syntax may seem more ergonomic.

…"${VAR_NAME}" Using "${VAR_NAME:null}", instead, would define the literal string "null".

…parameters No functional changes. This is in preparation of a future commit by Paul Tiedke.

muxator · 2020-04-21T01:44:03Z

I am going without introducing the coercition "null" -> null.

I am adding this snippet in settings.json.template, hoping it clarifies what is supported and what is not.

If you want to use an empty value (null) as default value for a variable,
simply do not set it, without putting any colons: "${ABIWORD}".

The settings.json.docker file in this PR is going to be updated accordingly.

…lude every single variable a driver may want

In this way, we also gain an explicit place for the default setting (still not filled in). No functional changes. This is in preparation of a future commit by Paul Tiedke.

muxator · 2020-04-21T02:41:47Z

Mentioned in DB_TYPE that the default is "not set, thus will fall back to DirtyDB (please choose one instead)"

muxator · 2020-04-21T02:45:01Z

This PR is really useful.

Many thanks for taking the time to prepare and update it.

Pulled in.

muxator · 2020-04-23T02:06:56Z

@sapkra: I have discovered that the documentation generator that Etherpad uses (https://github.com/markedjs/marked) is an ancient version, and does not render Markdown tables at all.

See what happens to your wonderful docker documentation:

Updating the library helps, but it's still horrible:

I stumbled on this issue while working on another documentation PR (#3921). If you are willing to help, you are welcome to drop a line there.

sapkra · 2020-04-23T02:13:24Z

Sorry but I have to say that I'm not even using etherpad yet. But I may will in the future and will create a few PRs then.

I just created these PRs to cleanup the docker-jitsi-meet repository a bit.

muxator · 2020-04-23T02:14:57Z

Ok no problem, thanks!

Consider this just a follow-up while we are moving towards the release.

sapkra · 2020-04-23T02:16:01Z

Maybe you can use something like docusaurus 2.0 to prettify the docs and/or website.

This change is needed because in 1.8.3 we are going to introduce Markdown tables in the documentation (#3873 and #3921), and the old marked version did not support generating them. Instead of committing the marked source code here, we live install from npm if needed via the Makefile. n.b.: at the time of this change, marked latest version is 1.0.0, released a few days ago. I am updating to the version immediately before that (0.8.2), because in 1.0.0 the hyperlinks in the Table of Contents do not work (probably a bug in that version).

This was referenced Apr 16, 2020

support control of log level for all components jitsi/docker-jitsi-meet#434

Closed

etherpad: use official image and making skin full width jitsi/docker-jitsi-meet#341

Merged

muxator added the docker label Apr 16, 2020

muxator added this to the 1.8.3 milestone Apr 16, 2020

sapkra force-pushed the config-env-vars branch from ac8c3a4 to fa43dce Compare April 17, 2020 23:44

muxator force-pushed the develop branch from 2fbfacc to 8956efc Compare April 18, 2020 16:11

sapkra force-pushed the config-env-vars branch from fa43dce to 412d66c Compare April 19, 2020 17:24

docker: make settings fully configurable via env vars

a259c5b

Now every setting in the official Etherpad container will be configurable via environment variables.

muxator reviewed Apr 19, 2020

View reviewed changes

Comment thread doc/docker.md Outdated

muxator force-pushed the config-env-vars branch from 412d66c to a7ddf26 Compare April 19, 2020 21:19

muxator force-pushed the config-env-vars branch from a7ddf26 to 1140daf Compare April 19, 2020 21:37

muxator reviewed Apr 19, 2020

View reviewed changes

muxator added 2 commits April 21, 2020 03:40

settings: clarify that null defaults are supported, using the syntax …

87b0995

…"${VAR_NAME}" Using "${VAR_NAME:null}", instead, would define the literal string "null".

docker: in the documentation, reorganize categorically the available …

82afbdb

…parameters No functional changes. This is in preparation of a future commit by Paul Tiedke.

muxator force-pushed the config-env-vars branch 2 times, most recently from 4065d0b to c7a3ba5 Compare April 21, 2020 02:04

muxator added 3 commits April 21, 2020 04:24

docker: in the db documentation, specify that we are not going to inc…

839656c

…lude every single variable a driver may want

docker: switch to table layout in the docs about the available settings

a07bf46

In this way, we also gain an explicit place for the default setting (still not filled in). No functional changes. This is in preparation of a future commit by Paul Tiedke.

docker: fill in the default values in the documentation

22fa601

muxator force-pushed the config-env-vars branch from c7a3ba5 to a259c5b Compare April 21, 2020 02:34

muxator changed the title ~~docker: make configuration fully configurable via env vars~~ docker: make settings fully configurable via env vars Apr 21, 2020

muxator merged commit 85adaa4 into ether:develop Apr 21, 2020

muxator mentioned this pull request Apr 23, 2020

DOCS: Cookies section - https://github.com/ether/etherpad-lite/issues/3563 #3921

Merged

muxator mentioned this pull request Apr 24, 2020

doc: remove old vendorized marked module and replace with 0.8.2 #3929

Merged

Uh oh!

Conversation

sapkra commented Apr 16, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

muxator commented Apr 16, 2020

Uh oh!

flecno commented Apr 17, 2020

Uh oh!

flecno commented Apr 17, 2020

Uh oh!

muxator commented Apr 19, 2020

Uh oh!

sapkra commented Apr 19, 2020

Uh oh!

muxator commented Apr 19, 2020

Uh oh!

Uh oh!

muxator commented Apr 19, 2020

Uh oh!

muxator Apr 19, 2020

Choose a reason for hiding this comment

Uh oh!

muxator left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

muxator commented Apr 21, 2020

Uh oh!

muxator commented Apr 21, 2020

Uh oh!

muxator commented Apr 21, 2020

Uh oh!

muxator commented Apr 23, 2020

Uh oh!

sapkra commented Apr 23, 2020

Uh oh!

muxator commented Apr 23, 2020

Uh oh!

sapkra commented Apr 23, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

sapkra commented Apr 16, 2020 •

edited

Loading

muxator left a comment •

edited

Loading