I can't really agree on the exceptionality of not including a config file,
my preference in most cases would be to WARN as most gems for third party
services do (e.g. NewRelic doesn't prevent my server from starting up if it
is missing it's configuration). I also want to make sure I understand this,
we want to prevent the server from starting if the file isn't there and we
want this to be the only option outside of manually checking for it's
existence?
In the end what I'm suggesting is that this developers should not be
limited in how they can handle a missing config. Providing a helper would
some kind of solution, but I don't see why it would be preferred over an
idiomatic solution similar to Hash#fetch. I would further suggest that it
suffers from the same problem of a typo issue, as well as being less "DRY"
since you have to repeat the name of the file. For example:
if has_config_for? :rdis do
config.x.redis = config_for(:redis)
else
config.logger.warn("Redis config is missing")
end
This is now misleading as it isn't redis.yml that is missing, but rdis.yml.
The block approach having a couple of benefits over just checking existance.
1) If a block or value is not provided as the second argument it can
basically keep the exact same logic (raising an exception) as before. No
change to any existing usages.
2) It can be made DRYer since we only reference the config file name once
and we could pass the name into the block if we want to allowing for a
generic helper that does something like logging a missing file warning or
error.
# /config/initializers/redis.rb
redis_config = Rails.application.config_for(:redis) do |config_name|
config.logger.warn("Configuration for #{config_name} not found!")
false
end
return unless redis_config
3) It feels more idiomatic to me as this is how Hash#fetch already works
4) An existence checking helper still feels like basically just another
form of nil checking in the end
On Wednesday, 28 October 2015 13:07:25 UTC-7, Rafael Mendonça França wrote:
>
> I see the file not being there an exceptional case. Making config_for not
> raise if the file doesn't exists can lead to more problems like not reading
> the config file because of a typo in the file name.
>
> ```
> if config_for(:rdis)
> # configure the thing
> else
> # don't configure the thing
> end
> ```
>
> The block version would be weird too:
>
> ```
> redis_config = config_for(:redis) do
> # don't configure the thing
> end
>
> Redis.config = redis_config
> ```
>
> The only solution that would work is the helper to check the file
> existence.
>
>
> On Wed, Oct 28, 2015 at 5:38 PM Chris Nicola <[email protected]
> <javascript:>> wrote:
>
>> Currently `Application#config_for` raises an exception whenever a config
>> file isn't found. However, this doesn't really seem very exceptional to me
>> and there are going to be a lot of times where I want to configure an
>> application module only if the configuration is available and something
>> else if it isn't.
>>
>> I could of course check if the file exists on my own, but then that
>> almost entirely defeats the convenience of this convenience function.
>>
>> A few possible solutions:
>>
>> #### Offer an easy way to check if the file exists like `config_exists?`
>>
>> I don't like this because it feels too much like a nil checking pattern.
>>
>> #### Allow config_for to accept a block or argument
>>
>> This is similar to the `Hash#fetch` pattern, and the value of the block
>> can be returned to the caller. The default exception behavior can remain if
>> no value or block is provided. This would also allow for a few patterns.
>>
>> 1. Provide alternative configuration logic when the file isn't found
>> 2. Don't configure the component when the file isn't found and log a
>> warning
>>
>> For example:
>>
>> ```
>> if config_for(:the_thing, false)
>> # configure the thing
>> else
>> # don't configure the thing
>> logger.warn "The thing is not configured, no configuration file is found
>> end
>> ```
>>
>> I'm definitely open to other suggestions, but the way `config_for` works
>> right now I'm very hesitant to use it.
>>
>> --
>> You received this message because you are subscribed to the Google Groups
>> "Ruby on Rails: Core" group.
>> To unsubscribe from this group and stop receiving emails from it, send an
>> email to [email protected] <javascript:>.
>> To post to this group, send email to [email protected]
>> <javascript:>.
>> Visit this group at http://groups.google.com/group/rubyonrails-core.
>> For more options, visit https://groups.google.com/d/optout.
>>
>
--
You received this message because you are subscribed to the Google Groups "Ruby
on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
Visit this group at http://groups.google.com/group/rubyonrails-core.
For more options, visit https://groups.google.com/d/optout.
