Skip to content

Feature/env var based config#42

Merged
thanaParis merged 7 commits intorc-1.1.0from
feature/env-var-based-config
Feb 21, 2024
Merged

Feature/env var based config#42
thanaParis merged 7 commits intorc-1.1.0from
feature/env-var-based-config

Conversation

@ChrisWeissmann
Copy link
Copy Markdown
Collaborator

Making the variables that are used to connect to the database, cached etc configurable via environment variables. Needed to do some extra work to make sure they aren't case sensitive, since there is direct mapping to the field names in the javascript object. Environment variables should be prefixed with CITRINEOS_ Configuring the message broker would look like: CITRINEOS_UTIL_MESSAGEBROKER_AMQP:...
Dots in the object are _ in the environment variables.

`

Copy link
Copy Markdown
Collaborator

@thanaParis thanaParis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The process appears to be, look at the environment for variables and then use those preferentially over the values in the system config from the code, correct?
I did previously dabble in something similar to this here.
It checks for an environment variable if there is a value missing in the system config, specifically for the db user/pass.

I think we should remove my take above, it doesn't make sense to check for environment variables twice. However, I think the new solution should go in its own file in the same folder as the above in Base. I think it should to apply to all configs, not just the new one you made. We should add a javadoc to the defineConfig method that explains this behavior.

We should apply this new function inside defineConfig where the old logic I wrote is. This way, environment variables can apply before the config is validated, permitting required sensitive info like user/pass to be set in environment variables rather than code.

@ChrisWeissmann
Copy link
Copy Markdown
Collaborator Author

Yea, I like that approach, basically putting it a bit more upstream and deeper into where the config is being defined. I will give it a try.

Copy link
Copy Markdown
Collaborator

@thanaParis thanaParis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved!

@thanaParis thanaParis merged commit 0107b23 into rc-1.1.0 Feb 21, 2024
@ChrisWeissmann ChrisWeissmann deleted the feature/env-var-based-config branch February 22, 2024 09:13
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