piotrmurach/tty-config

Feature Request: Optionally read configuration from ENV vars

fwolfst opened this issue · 6 comments

This is not my words, but @piotrmurach , cited from #1 (I did not know beforehand what github will make out of it if one clicks on "New issue" from a single post). So here it comes:

Your idea is pretty neat about adding env vars configuration so I would like for tty-config to support it. Given your description my current thinking around api goes like this:

config.env_prefix = 'mytool'  # internally always capitalised

config.envs << "host" # add manually envs your are interested in
config.envs << "port"

ENV['MYTOOL_HOST'] = '192.168.1.17' # these would normally be from console
ENV['MYTOOL_PORT'] = '7727' # but for the sake of his example

config.fetch(:host)   # fetch is smart enough to look in env first
# => '192.168.1.17'
config.fetch(:port)  # keys are case insenstive
# => '7727'

Apart from manually specifying envs you're interested in, there could be an automatic way:

config.env_prefix = 'mytool'
config.env_autoload  # loads all env variables with 'mytool' prefix

ENV['MYTOOL_HOST'] = '192.168.1.17'
ENV['MYTOOL_PORT'] = ' 7727'

config.fetch(:host) 
#=> '192.168.1.17'
config.fetch(:port)
# => '7727'

Probably the most important point is that the fetch would read envs everytime it's called! Given that user my call a tool sometimes with env or not, it would be dangerous to save away values.

The one thing I'm not entirely sure about is how to handle 'complex' env vars:

ENV['MYTOOL_FOO_BAR'] = '192.168.1.17'

config.fetch(:foo_bar) # this is the one I prefer which preserves exactly the env name
config.fetch(:foobar) # potentially collpase any separators

The precedence I would follow in determining what value to retrieve first with fetch call, where next item overwrites the previous:

1. default  # nothing specified anywhere, just take default
2. config
3. ENV
4. call to set  # most powerful, direct call always overwrites anything set prior

What do you think about the env vars API?

Originally posted by @piotrmurach in #1 (comment)

Sounds good to me. I'd also opt for as little magic in name-transformation as possible, so underscores stay underscores. (MYTOOL_FOO_BAR -> foo_bar). And clearly state that only "first-level" settings can be set by ENV by now.

Btw discourse does it like this: https://github.com/discourse/discourse_docker/blob/2791af602b85edf46dc06bdbb754390346166d8b/templates/web.template.yml#L49

I have fully implemented the above specification, please see set_env for more details. As a bonus I have added ability to bind deeply nested configuration keys with ENV variables names:

config.env_prefix = 'mytool'
config.set_env(:foo, :bar, :baz) { "HOST" }

config.fetch(:foo, :bar, :baz) # => '192.168.1.17'
config.fetch('foo.bar.baz') # => '192.168.1.17'

You can checkout master branch and see what you think. I hope you like it!

Awesome! But thinking about it, set_env might be a bit misleading, as it suggests that a environment variable is set:

config.set_env(:world) { 'world' }
cmd = TTY::Command.new
cmd.run('echo hello $WORLD')

People might assume set_env sets an ENV variable e.g. for subshells.
Probably accept_env would be the better naming choice (but I am not a native speaker).

I can see your concern. I've spent quite a bit of time thinking about the best name for the api. When I'm adding new calls I think how they align with the current api. In this case, I already have set call which sets a configuration option name to a value. You can have deeply nested configuration option:

config.set(:foo, :bar) { 'baz' }

The decision to use set_env is based on the fact that the receiver of a message is a configuration instance and what we're doing is setting a configuration option to the value of environment variable. So in this context set_env is appropriate. It's akin to saying set option from env variable as this is what we're actually doing. It feels perfectly natural to say:

config.set_env(:foo, :bar) { 'baz' }  # => BAZ is our env variable

I'm not sure that people would assume that env is set. If the receiver was for example cmd as in your example then that would be perfectly acceptable assumption. The cmd.set_env is definietly implying that environment var is set for the command to run. However, in the context of configuration object that is rather unlikely.

Potentially to better express intention behind associating configuration settings with env variables:

config.set_from_env(:foo, :bar) { 'baz' }

@fwolfst I've changed set_env to set_from_env to better reveal intention. Any thoughts?

I'm ready to release this version with all the environment vars support, new ini file type configs loading etc... Now is probably best time to raise any concerns or give your blessing.

Thanks for all your suggestions, they definietly made this library more complete and useful!