-
Notifications
You must be signed in to change notification settings - Fork 0
Config and environment files #43
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
Conversation
|
Two comments:
|
|
I guess here we can and all env vars needed for postgresql, mongo, minio, aws (tipical endpoint and credentials) as well as fda would need (port, .. ). |
I agree In general, all that now is hardwired in the code should be exposed in configuration. |
Co-authored-by: Alvaro Vega <[email protected]>
| @@ -0,0 +1,56 @@ | |||
| # Configuration | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file should be linked from main README.md (documentation section)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added in commit f00af80
doc/config.md
Outdated
| - [Introduction](#introduction) | ||
| - [Variables](#variables) | ||
| - [Environment](#environment) | ||
| - [Postgre](#postgre) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Postgre, Postgres or PostgreSQL? Which one is the proper term?
(I tend to think it is PostgreSQL)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well after research apparently postgre is the only one no one uses. I thought it was the correct abbreviation but Postgres was the original name of the product so its what's usually used. Changed to PostgreSQL in the commit 341e5f2.
.env.example
Outdated
| FDA_POSTGRE_CLIENT_USER=exampleUser | ||
| FDA_POSTGRE_CLIENT_PASSWORD=examplePass | ||
| FDA_POSTGRE_CLIENT_HOST=exampleHost | ||
| FDA_POSTGRE_CLIENT_PORT=5432 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we should use
FDA_PG_...
?
as we did recently in Kafnurs (e.g. KAFNUS_TESTS_PG_USER)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed in commit 608ada7
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM and pass ball to @fgalan
Co-authored-by: Fermín Galán Márquez <[email protected]>
fgalan
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
#14
Add config and .env files.