apply env to array elements #350
Conversation
|
@jonseymour Thanks for the PR, I know I don't have any tests around config parsing, but could you add a test that demonstrates the new behavior? Don't worry about testing everything else just the changes. We will get to the other tests in time. |
|
Ok, I'll add tests. I'll have to abstract the call to os.GetEnv() to allow tests to mock this call, as required. |
jonseymour
force-pushed
the
jss-348-apply-env-to-array-elements
branch
from
8f2928b to
0e18d04
Compare
|
@nathanielc I've extended a test and done a more complete reorganisation to simplify the structure of the applyEnvOverrides function. |
|
Actually, I missed an additional simplifcation that is now possible. I'll re-roll. |
…lements The refactor splits the handling of structs into a separate function and adds a possibly empty fieldDesc parameter to applyEnvOverrides to allow applyEnvOverrides to handle simple values as well as structs. Signed-off-by: Jon Seymour <[email protected]>
2nd parent of merge, merges cleanly with v0.10 branch.
jonseymour
force-pushed
the
jss-348-apply-env-to-array-elements
branch
from
0e18d04 to
c92aa78
Compare
|
@jonseymour Just to make sure, you made that last simplification correct? Also can you add a changelog entry? Otherwise this looks good to go. Thanks! |
Signed-off-by: Jon Seymour <[email protected]>
|
Yes, simplifications complete. Changelog now updated. |
|
Nice, this should remove sed magic from my Docker image, thanks! |
…ements apply env to array elements
This fixes #348.
In particular:
KAPACITOR_INFLUXDB_0_URLS_0=http://localhost:9000 kapacitord configshould now produce a config which does the expected thing.