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

Add Environment Variable concept #97

Open
wants to merge 14 commits into
base: master
Choose a base branch
from

Conversation

8ear
Copy link

@8ear 8ear commented Feb 28, 2021

Hi,
as mentioned in #19 and in #88.
The following is my proposal for an environment variable concept.

Please review and add feedback how you find it?

Kind regards

@garrit-schroeder
Copy link

Having a more variable spoken approach sound good.
Only naming convention will break my current PR #88
This one is especially designed to allow setting MISP variables via ENVs in a dynamic way. Not to be put in a strict vars.sh file. There are hundreds of settings that can be modified using cake CLI.

If we can integrate both PRs its fine with me.

And another thing to point out is that changing all existing environment variable names will break all existing setups of this project. So this might actually end up in a lot of work.

@8ear
Copy link
Author

8ear commented Mar 3, 2021

Having a more variable spoken approach sound good.
Only naming convention will break my current PR #88
This one is especially designed to allow setting MISP variables via ENVs in a dynamic way. Not to be put in a strict vars.sh file. There are hundreds of settings that can be modified using cake CLI.

That was not my intention of the default_vars.sh. The intention of that file is to define default values for variables which should be always set. Nothing more.

If we can integrate both PRs its fine with me.

Yeah I think this will be a great idea. But I am not so aware with you code. So can you support me here so that we can get both PR together?
I added already comment on your PR but I am unsure if this will work.

And another thing to point out is that changing all existing environment variable names will break all existing setups of this project. So this might actually end up in a lot of work.

What we can do is to support the existing ENV vars a few versions long and map it internally in the entrypoint to the new one. So I think this should not be a big problem.

@8ear 8ear changed the title Add Environment Variable concept [WIP] Add Environment Variable concept Mar 28, 2021
@8ear
Copy link
Author

8ear commented Mar 28, 2021

It looks that the cake settings does not work.
I need to check why this does not be recognized.

@8ear
Copy link
Author

8ear commented Mar 30, 2021

It looks that the cake settings does not work.
I need to check why this does not be recognized.

I fixed it.

@8ear 8ear changed the title [WIP] Add Environment Variable concept Add Environment Variable concept Mar 30, 2021
@coolacid
Copy link
Owner

I'm going to need to look at this a few more times. Also, will need a rebase - and check Codacy issues.

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