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

Set key to file url instead of file name to allow config override files with the same names #842

Merged
merged 1 commit into from Mar 11, 2019

Conversation

Projects
None yet
2 participants
@hamdanjaveed
Copy link
Collaborator

commented Mar 11, 2019

For e.g., if we had the following override files when loading our test_app's misk config

/resources/db_config/test_app-config.yaml
/resources/cache_config/test_app-config.yaml

We would like to override our config with the content from both files. Currently, we only
use the filename as a key when creating a mapping of our config. This PR changes the key
to contain the entire URL.


val config = MiskConfig.load<TestConfig>("test_app", defaultEnv, overrides)
assertThat(config.consumer_a).isEqualTo(ConsumerConfig(14, 1))
assertThat(config.consumer_b).isEqualTo(ConsumerConfig(34, 79))

This comment has been minimized.

Copy link
@wesleyk

wesleyk Mar 11, 2019

Collaborator

oh we should make sure these assertions check values from both resources

This comment has been minimized.

Copy link
@hamdanjaveed

hamdanjaveed Mar 11, 2019

Author Collaborator

Just confirmed, they do

Hamdan Javeed
Set key to file url instead of file name to allow config override fil…
…es with the same names

For e.g., if we had the following override files when loading our test_app's misk config

/resources/db_config/test_app-config.yaml
/resources/cache_config/test_app-config.yaml

We would like to override our config with the content from both files. Currently, we only
use the filename as a key when creating a mapping of our config. This PR changes the key
to contain the entire URL.

@hamdanjaveed hamdanjaveed force-pushed the hamdan-fix branch from 24bed10 to c654d4e Mar 11, 2019

@wesleyk wesleyk merged commit dd79a38 into master Mar 11, 2019

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.