Fix undefined method error at EnvironmentLoader #175
Conversation
|
This is not a reasonable way to solve this problem. You are correct in your assertion that 3.2 doesnt define the method: https://github.com/rails/rails/blob/3-2-stable/activesupport/lib/active_support/core_ext/hash/keys.rb But calling methods on hashes till any specific one of them fails, so we redefine them all is not a good idea. Instead bump the version of active support, or add a check to the version of active support which has the problem and define it conditionally if the version does not bring in the correct method. Please try again. |
| def deep_symbolize_keys | ||
| keys.each do |key| | ||
| value = delete(key) | ||
| self[(key.to_sym rescue key) || key] = value |
There was a problem hiding this comment.
Avoid using rescue in its modifier form.
|
It is good idea that I add a check to the version of active support, but active support is not written in gemspec. So, Therefore, I add |
ActiveSupport::VERSION::MAJOR # 3
ActiveSupport::VERSION::MINOR # 2
# e.g. 3.2 |
|
We should not use … |
|
@h3poteto @Senjai actually I think we don't need key: "value"Not wrapping the keys with quotes: "key": "value"So, maybe the fix would be to remove |
|
YAML parser did not construe key as symbol, if YAMLs are written in this way: key: "value"and YAML.load("aws:\n access_key_id: hoge\n")
=> {"aws"=>{"access_key_id"=>"hoge"}}But, when YAMLs are written in this way: :key: "value"
YAML.load(":aws:\n :access_key_id: hoge\n")
=> {:aws=>{:access_key_id=>"hoge"}}So, if everyone use symbol syntax for yaml like this: :key: "value"we can remove |
|
@h3poteto interesting, great point! Because of this - I thought that symbol as the default, as Sidekiq uses But based on this:
I would remove wdyt? |
|
@phstc Thank you for proposal. YAML.load("aws:\n access_key_id: hoge\n")
=> {"aws"=>{"access_key_id"=>"hoge"}}I considered this. I thought that like below:
but, now I think that we can remove Maybe, developers will read README, when developers start to use shoryuken. So, developers will detect new :aws:
:access_key_id: hogeI think that symbol syntax is fine with developers. What do you think? |
|
hi @h3poteto I personally prefer: aws:
access_key_id: hogeBut in favor of styling preferences in something that widely used Thanks a lot for your PR and all researched! 🍻 |
When I use
Shoryuken::EnvironmentLoader#loadin Rails3, I got an raised error below.This is because ActiveSupport v3.2 does not define
Hash#deep_symbolize_keys.So, I've added closely check hash methods in
core_ext.rb.