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

ConfigFile: settings.foo fails when settings file contains non-implemented environments #1243

Closed
JonMidhir opened this issue Jan 27, 2017 · 3 comments

Comments

@JonMidhir
Copy link
Contributor

JonMidhir commented Jan 27, 2017

I spent a while debugging this today in an older application. If your settings file contains more 'environments' than your application supports it will fail in every environment in quite an opaque way.

Symptom:

Calling settings.foo fails with

NoMethodError - undefined method `foo' for [ApplicationClass]:Class:

Reproducing:

The following will break settings in every environment, unless the application happens to support an environment called 'default':

default: &default
  foo: bar

development:
  <<: *default

production:
  <<: *default

test:
  <<: *default

Solution:

The code is here:
https://github.com/sinatra/sinatra/blob/master/sinatra-contrib/lib/sinatra/config_file.rb#L159

I believe this is actually intended to do the opposite, to check that every environment in the application is covered in the settings file - not that every environment in the settings file is honoured by the application. Even then it could fail more gracefully; for example with an error message to warn the user of missing environments in the settings file or a suitable exception if it's the current environment.

I can code this up over the weekend if it'd be useful. As a bonus it'd allow the functionality seen above, where a common, environment-agnostic set of default settings can be used.

Workaround:

In the mean time you can get around this problem by passing the current environment to settings manually and accessing the information as a hash:

settings.send(settings.environment)['foo']
@JonMidhir JonMidhir changed the title ConfigFile could fail ConfigFile: settings.foo fails when settings file contains too many environments Jan 27, 2017
@JonMidhir JonMidhir changed the title ConfigFile: settings.foo fails when settings file contains too many environments ConfigFile: settings.foo fails when settings file contains non-implemented environments Jan 28, 2017
@zzak
Copy link
Member

zzak commented Jan 30, 2017

Need to give this a second look before it can be prioritized

@JonMidhir
Copy link
Contributor Author

No worries, it's a bit hairy alright. Let me know if I can help.

@namusyaka namusyaka added this to the v2.0.2 milestone Feb 19, 2018
@namusyaka namusyaka modified the milestones: v2.0.2, v2.0.3, v2.0.4 Jun 5, 2018
@namusyaka namusyaka modified the milestones: v2.0.4, v2.0.5 Sep 14, 2018
@namusyaka namusyaka modified the milestones: v2.0.5, v2.0.6 Dec 22, 2018
@jkowens
Copy link
Member

jkowens commented Mar 19, 2020

Fixed by #1244.

@jkowens jkowens closed this as completed Mar 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants