Type Conversion of Environment Variables in Redis Cache Configuration

rubyonrails-core
1

Hi all,
In my project, I’m using Redis as the cache_store .
However, I noticed a behavior that concerns me, and I would like to hear your opinions.

I’m using environment variables to specify the configuration values such as pool_size for the cache_store. However, I encountered the following error unless the values are converted to numeric types

# config/environments/development.rb
config.cache_store = :redis_cache_store, {
  url: ENV.fetch('CACHE_REDIS_URL'),
  pool_size: ENV.fetch('CACHE_REDIS_POOL_SIZE', 5),
  :
}

[1] pry(main)> Rails.cache.write("/foo", "bar")
/usr/local/bundle/gems/connection_pool-2.3.0/lib/connection_pool/timed_stack.rb:67:in `+': String can't be coerced into Float (TypeError)

https://github.com/mperham/connection_pool/blob/main/lib/connection_pool/timed_stack.rb:67

It seems that the values retrieved from the environment variables remain as strings, which causes the above error. Of course, the values retrieved from environment variables are of string type. This issue can be resolved by using to_i .

# config/environments/development.rb
config.cache_store = :redis_cache_store, {
  url: ENV.fetch('CACHE_REDIS_URL'),
  pool_size: ENV.fetch('CACHE_REDIS_POOL_SIZE', 5).to_i,
  :
}

However, I don’t think it’s appropriate that the error occurs only when writing to the cache if a string value is specified. Perhaps it would be better if an error occurred when a string value is provided.

How do you feel about it? We look forward to hearing your thoughts.
Thanks for everyone’s efforts on Rails!

2

Yes, it’s not great.

There are two competing concerns here though. The cache store ideally shouldn’t try to connect to the cache server during boot so that the app can boot even if the cache is down.

But yes, we should boot if the parameters are clearly invalid.

But I’m sure we can improve things significantly. Feel free to file this as an issue on GitHub.

3

What if we make it raise an error if the Rails env is development?

4

Thank you for your response.

I see, I wasn’t aware of those concerns.
In that case, the approach of causing an error that prevents Rails from booting may not be ideal.
We need to consider minimizing the impact on Rails startup, perhaps by simply outputting a warning log.

Thank you. I will create an issue on GitHub.

5

Thank you for your response.
I see, I didn’t come up with that idea.

As a concern, Rails provides configuration files for each environment under /config/environments.
It is conceivable to have a scenario where the cache store is not used in the development environment for cost-saving purposes, but only used in the staging and production environments.

In that case, I feel that there is a possibility of not being able to detect this issue for behaviors specific to certain environments.

6

Yea agreed, I think converting to an integer and adding a Rails log warning is enough when the value is a string. Currently my app uses Figaro gem to read the ENV variables and I got this warning How to disable 'WARNING: Use strings for Figaro configuration'? · Issue #248 · laserlemon/figaro · GitHub

WARNING: Use strings for Figaro configuration. false was converted to "false".
WARNING: Use strings for Figaro configuration. 120 was converted to "120".
7

Oh, it’s very helpful to have an existing implemented application as a reference.
I once again realize that having warning logs like the implementation in Figaro would be desirable.
Thank you for the wonderful information :smile: .

8

Hi all. I just created an issue on GitHub.
This is my first time creating an issue on the Rails GitHub repository, so I’m feeling a bit nervous.

Thank you for your feedback !.