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

Allow a global prefix for config field names #8

Closed
wants to merge 1 commit into from

Conversation

cbndr
Copy link

@cbndr cbndr commented May 7, 2021

Hi Jeremy,
I like your config package a lot, minimalist and still covers 99% of the config use cases.

One thing I am missing is using a global prefix for field names. I deploy to k8s and in our environment, a large number of env vars are automatically injected into containers. It is very helpful therefore, to prefix env vars for instance with the app name. Also, with a prefix there is no risk using a prexisting (injected) name. Example:

MYAPP__DATABASE_PORT=5432

Would be used like

var c MyConfig
err := WithPrefix("MYAPP").From("config").FromEnv().To(&c)

Added "WithPrefix" funcs and a test for that.

Best regards, Chris

@JeremyLoy
Copy link
Owner

HI @cbndr, sorry for the late reply. I did not get an email for some reason. I'll check my settings.

os.Setenv("SUBCONFIG__IPWHITELIST", "0.0.0.0 1.1.1.1 2.2.2.2")

Does the above not fit your use case already? If not I'll consider adding another method

@cbndr
Copy link
Author

cbndr commented Jul 7, 2021

No worries =)
So you are suggesting to encase all config with a struct carrying the name of the app as a work-around?
TBH I'd prefer having a few LOCs for the prefix, as it is also more expressive when reading the code.

@JeremyLoy
Copy link
Owner

@cbndr I see your point now, I agree there would be a lot of benefit here.

What do you think about having a different To method? I would like to only parse config into memory a single time. So something like this.

builder := config.FromEnv()
configA := builder.ToOverload(&a, "prefixA")
configB := builder.ToOverload(&b, "prefixB")

@cbndr
Copy link
Author

cbndr commented Jul 9, 2021

Sure, that could work as well. Although I think the method name "ToOverload" is very self-explanatory. On top of that, From() could accept an additional variadic param to allow for an optional prefix.

@JeremyLoy
Copy link
Owner

Oh haha I didn’t literally mean ToOverload , just wanted to make it clear what it was

I don’t want to mess with From, since that is doing all the parsing. What about adding the variadic arg to To?

@JeremyLoy
Copy link
Owner

Added in v1.4.0!

@JeremyLoy JeremyLoy closed this Jul 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants