feat(service): add a config package to load application configuration using Viper #27
Conversation
service/configs/espressoconfig.yaml
Outdated
| db: | ||
| host: "localhost" | ||
| port: 3308 |
There was a problem hiding this comment.
3306? check dockercompose
There was a problem hiding this comment.
As I was running Espresso service in my local machine (in debug mode) and not as a docker container, I had mapped the host as localhost and port as 3308 instead of 3306. I will resolve this error and push it.
service/internal/config/config.go
Outdated
| ) | ||
|
|
||
| func Load() (model.Config, error) { | ||
| viper.AutomaticEnv() |
There was a problem hiding this comment.
move to service/internal pkg
There was a problem hiding this comment.
service/internal/pkg/config/config.go ?
| "github.com/Zomato/espresso/lib/s3" | ||
| ) | ||
|
|
||
| type Config struct { |
There was a problem hiding this comment.
move this to service/internal pkg
There was a problem hiding this comment.
service/internal/pkg/config/model/config.go ?
There was a problem hiding this comment.
..pkg/config/model.go for models and pkg/config/config.go for logic
|
related to #25 |
|
@Ashish-walkingn8mare A lot of dependencies are being created at the time of handling requests which should be created at the time of application startup. For example,
Possible Solution:
Possible Solution: |
fe9c180 to
f18c11d
Compare
|
@Ashish-walkingn8mare how should we proceed? |
|
@akkahshh24 lets first close the issue at hand, this can be a new issue, although service is only made as a demo, its not necessary to add these improvements |
|
@akkahshh24 lmk when changes are ready to review, please test throughly before that |
|
@Ashish-walkingn8mare I will pass mySQL DSN and other parameters to these functions to get rid of viper.GetString() at all places. Will let you know once done. |
|
@akkahshh24 I see what you were saying earlier, passing these env variables like DSN into the functions is not correct. For Prob1- Initilize all the certificates from the config at init into the espresso service. Make sure to add support for new certificate config keys in the same format |
|
@Ashish-walkingn8mare Added Credential Store. |
|
@Ashish-walkingn8mare any changes to be made? |
What does this PR do?
Introduces a new
configpackage that loads application configuration from a YAML fileespressoconfig.yaml8000 at startup.Why is this change necessary?
Centralizing configuration management makes the application more maintainable.
Loading configuration from a YAML file allows easier updates without modifying code.
Module
serviceRelated Issues
Related to #25