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

Make LDAP configurable in the container image #4144

Closed
wants to merge 2 commits into from

Conversation

jcgruenhage
Copy link
Contributor

No description provided.

@jcgruenhage
Copy link
Contributor Author

Once again don't get how the CI failure is related to the PR. From my testing, this works, I just logged into a server using this.

@jcgruenhage jcgruenhage requested a review from a team November 3, 2018 02:37
@richvdh
Copy link
Member

richvdh commented Nov 5, 2018

I recently rejected #3284 on the grounds that adding a bajillion environment vars to the container setup wasn't scalable and at some point people should just write a yaml file themselves.

I'm willing to hear reasons that it's a good idea in this case though?

[and yes, the CI failure looks unrelated...]

@richvdh richvdh removed the request for review from a team November 5, 2018 13:16
@jcgruenhage
Copy link
Contributor Author

I agree that this isn't scalable, but I don't agree that writing the config file myself is the solution here. Other tools are completely configurable via env vars (see grafana for example: http://docs.grafana.org/installation/configuration/#using-environment-variables), having similar possibilities in Synapse would be nice.

This template stuff was one of the things I criticized back when the docker PR wasn't merged yet. Let's not merge this anyway, you are right that this is getting too large.

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.

2 participants