Skip to content
This repository was archived by the owner on Sep 25, 2020. It is now read-only.

Add datacenter support for common.json#26

Open
freeqaz wants to merge 1 commit into
masterfrom
common-datacenter-support
Open

Add datacenter support for common.json#26
freeqaz wants to merge 1 commit into
masterfrom
common-datacenter-support

Conversation

@freeqaz

@freeqaz freeqaz commented Jun 24, 2015

Copy link
Copy Markdown

Adds support for common.DATACENTER.json which is useful to reduce duplication of configuration.

r: @Raynos @lxe @rajeshsegu @malandrew

@Raynos

Raynos commented Jun 24, 2015

Copy link
Copy Markdown
Contributor

👎 what does common datacenter even mean.

datacenter is a production thing only; you only have a datacenter in production.

@freeqaz

freeqaz commented Jun 24, 2015

Copy link
Copy Markdown
Author

I keep hitting a wall when testing datacenter-specific stuff. The model that we're using makes it hard to test something, since we keep a test.json environment. So if I want to test, I've got to create test.datacenter.json and keep it in sync with production.datacenter.json. It makes it hard to ensure that the production config is well tested, because it's never actually asserted against.

Having common.datacenter.json allows production to utilize the same set of datacenter-specific config that the test environment is asserting against.

@lxe

lxe commented Jun 24, 2015

Copy link
Copy Markdown
Contributor

You can just overload the config for any wacky tests with --config=/var/config/some-file.json flag

@rajeshsegu

Copy link
Copy Markdown

I like the idea of common.pek1.json for the same reason of avoiding duplicates between both test.pek1.json and production.pek1.json.

@lxe tests needs to be written against real config data otherwise it would not be good tests.

@Raynos

Raynos commented Jun 24, 2015

Copy link
Copy Markdown
Contributor

@freeqaz test.json is a terrible idea; don't do it.

If you want to test stuff use the seedConfig feature; that's the correct way to test stuff.

We really should remove the test.json feature.

@freeqaz

freeqaz commented Jun 24, 2015

Copy link
Copy Markdown
Author

You would load production and then override the config with test-specific things?

I don't see seedConfig in the readme. I'll check out the source.

@Raynos

Raynos commented Jun 25, 2015

Copy link
Copy Markdown
Contributor

The option is called "seed". Not "seedConfig"

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants