amenezes / config-client

config-client package for spring cloud config and cloud foundry
https://config-client.amenezes.net/
Apache License 2.0
24 stars 17 forks source link

Supports config file #27

Closed luiscoms closed 4 years ago

luiscoms commented 4 years ago

This merge requests support to load config file by profile.

It loads "config/{profile}.yml" file by default. File path can be changed by setting CONFIG_FILE_PATH environment variable.

All configuration will be merged at de end.

lgtm-com[bot] commented 4 years ago

This pull request introduces 2 alerts when merging fcfdc7faad53d76df3944e7594115d5377bef79c into 8d0b34bdf90c8844b0a22a5cab691fb39cd96b5a - view on LGTM.com

new alerts:

lgtm-com[bot] commented 4 years ago

This pull request introduces 2 alerts when merging 52a0a8a5f17680f8aaedc5c411c0affc63ba41b6 into 8d0b34bdf90c8844b0a22a5cab691fb39cd96b5a - view on LGTM.com

new alerts:

lgtm-com[bot] commented 4 years ago

This pull request introduces 2 alerts when merging 3f760e195fbd30b58292a13e9b107c6deaa7e182 into 8d0b34bdf90c8844b0a22a5cab691fb39cd96b5a - view on LGTM.com

new alerts:

amenezes commented 4 years ago

Hello @luiscoms,

First, thanks for your time to help on improvements in the config-client.

I don't know, at first, if bring an option to manage configuration locally it's a good choice. I think this breaks a bit the externalized configuration logic. This has already briefly been discussed in another issue.

Originally I imagine that all we need it's to have a minimal configuration locally, just the necessary to contact the spring cloud config server and after recovering the configuration initialize the application properly.

What do you think about? Are there any use cases that you would like to cover?

luiscoms commented 4 years ago

Hi @amenezes,

This pull request cover the fail of config server and supports a default config that can be overridden by server config, just like spring boot does.

luiscoms commented 4 years ago

Any considerations?

lgtm-com[bot] commented 4 years ago

This pull request introduces 2 alerts when merging c9e159942769b21ce466d0163c58c8bcab81eacf into 8d0b34bdf90c8844b0a22a5cab691fb39cd96b5a - view on LGTM.com

new alerts:

amenezes commented 4 years ago

@luiscoms,

Could you explain me a little more about this feature and your needs?

There's some advantage in this approach over use, for example, of dynaconf or the configparser for manage local configurations?

luiscoms commented 4 years ago

Hi, the pull request allows to load config file if exists and merge with cloud config. I want to use config client to load both configs

amenezes commented 4 years ago

For now this issue will marked as invalid and closed. Maybe in the future we can reevaluate.