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

Database Connection - Use environment variables (optional setting) #4791

Closed
M-arcus opened this issue Apr 9, 2018 · 12 comments
Closed

Database Connection - Use environment variables (optional setting) #4791

M-arcus opened this issue Apr 9, 2018 · 12 comments

Comments

@M-arcus
Copy link

M-arcus commented Apr 9, 2018

I am currently creating a Dockerfile for a selfhosted server-solution (cloudron). During first testing, I have encountered the following problem:

Problem

The MySQL credentials change at every start of the docker container, meaning the container only works at the first run. But the MySQL credentials can be accessed per getenv().

Proposed solution:

Add a config setting 'Get MySQL-Connection from Environment Variables'
Set the default false, so it doesn't break anything for existing installations.
If set to true, the credentials will be taken from the Environment Variables per getenv().

I would open a PR for it, if you agree to adding this feature

@MrPetovan
Copy link
Collaborator

This is similar to #4784, except that it concerns the install process, while your proposed solution would be bypassing the install for MySQL config, am I right?

@M-arcus
Copy link
Author

M-arcus commented Apr 9, 2018

Exactly, my solution would only bypass the MySQL part of the installation, since the values will be dynamic. Only an initial connection check would be needed.

@MrPetovan
Copy link
Collaborator

Here's my question then: do we need to choose between either implementation or can we have both at the same time?

@M-arcus
Copy link
Author

M-arcus commented Apr 9, 2018

It should be possible to use both at the same time, I would build my approach on top of #4784 's approach, as an extra checkbox/parameter during installation which activates either the "normal" or the "getenv" mode.

@MrPetovan
Copy link
Collaborator

Then by all mean, submit a PR.

Repository owner deleted a comment Apr 10, 2018
@nupplaphil
Copy link
Collaborator

hm @M-arcus .. how do we sync changes from here and maybe #4784 to not break each other? How much is your progress so far? So shall I will wait with my approach/changes until you're finished?

@M-arcus
Copy link
Author

M-arcus commented Apr 20, 2018

Fixed by #4855

@M-arcus
Copy link
Author

M-arcus commented Apr 20, 2018

Documentation needed.
@MrPetovan Should I open a new issue for it?

@MrPetovan MrPetovan added the Documentation Needed Label for PRs that add new features that need to be documentated label Apr 20, 2018
@MrPetovan
Copy link
Collaborator

I just added the label, hopefully someone (maybe you?) will pick up on it.

@MrPetovan MrPetovan added this to the 2018.05 milestone Apr 20, 2018
@tobiasd
Copy link
Collaborator

tobiasd commented May 13, 2018

@M-arcus could you write the docs for this?

@M-arcus
Copy link
Author

M-arcus commented May 13, 2018

@tobiasd yes, will do

M-arcus added a commit to M-arcus/friendica that referenced this issue May 15, 2018
@MrPetovan MrPetovan removed the Documentation Needed Label for PRs that add new features that need to be documentated label May 15, 2018
@MrPetovan
Copy link
Collaborator

Thank you for your work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants