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 for configuration keys with nil values to fall back onto defaults #627

Closed
wants to merge 1 commit into from

Conversation

cmw
Copy link

@cmw cmw commented Jul 22, 2016

In our effort to turn our deployment into something 12-factor-compliant, we tried to base our configuration solely on ENV variables like so:

url: <%= ENV['REDIS_URL'].presence || nil %>
sentinels:
  - host: <%= ENV['REDIS_SENTINEL_HOST'].presence || nil %>
    port: <%= ENV['REDIS_SENTINEL_PORT'].presence || nil %>
role: <%= ENV['REDIS_ROLE'].presence || nil %>
timeout: <%= ENV['REDIS_TIMEOUT'].presence || 0.1 %>
db: <%= ENV['REDIS_DB'].presence || 0 %>
host: <%= ENV['REDIS_HOST'].presence || '127.0.0.1' %>
port: <%= ENV['REDIS_PORT'].presence || 6379 %>

This however results in the configuration hash holding several unnecessary keys with nil values. Thus these changes are needed so in case of a key existing but being nil, the default is still used.

I assume this was intended all along, but did only work for some of the keys, not all.

@cmw
Copy link
Author

cmw commented Sep 20, 2016

@badboy is there a chance to get this merged?

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.

None yet

1 participant