queue_storage: allow defining Resque configuration without sentinels

[miguelsorianod]

This allows to configure a Redis Resque without defining sentinels.
This is the case for example when Redis ElastiCache is used.

While at it also allow picking up the configuration in dev/test modes.

[unleashed]

@miguelsorianod IIRC you can still run the queues storage without sentinels if you set the value to an empty array

[unleashed]

@miguelsorianod can you please check whether this valid_cfg? method makes sense at all given that the client for this storage is now created by the same code for the client for the main storage, which should already error out if there is an inconsistent configuration?

[miguelsorianod]

About the possibility of using an empty array, that's possible because the cfg_sentinels_handler in storage.rb sees empty as not using sentinels:

apisonator/lib/3scale/backend/storage.rb

Line 190 in d6d4d10

return options if sentinels.nil? || sentinels.empty?

However I'd prefer not having to specify an empty array in the configuration file, which seems ugly to me.

About seeing if the valid_cfg? methods makes sense let me take a look 👍

[miguelsorianod]

Both Resque and Storage end calling the methods in storage.rb but the initialization for resque is a specific one in queue_storage.rb. There, it configures some parameters depending on the introduced configuration, which are different than the one for storage (for example, the 'default_url' redis parameter) , or renames the master_name parameter to 'url', which is indeed a Redis config-related parameter,and then indeed calls some storage helpers to generate a configuration.

I still see it makes sense having this method because it references several sublevels of configuration: cfg.queues.master_name and we would not want to receive a NoMethodError.

[unleashed]

I just took a look and we could actually do:

def self.connection(_env, cfg)
  init_params = {
    url: cfg.queues && cfg.queues.master_name,
    default_url: '127.0.0.1:6379'
  }
# [...]

...for removing the branching on env and the different configurations. I was about to tell you not to worry about it, as it's not important, but then I remembered these comments which would be nice to fix.

[miguelsorianod]

I agree. As we discussed I now pushed a change that allows dev/test environments to specify a non-localhost URL.

Furthermore, we now do not crash the program on the storage_queue.rb file anymore but we let the storage.rb to do the crashing. I've updated the tests accordingly.

[unleashed]

typo space between identifier and }?

[miguelsorianod]

fixed

[miguelsorianod]

This has been changed because the RuntimeError is not raised anymore after the changes that have been done in the program.

We know that it should crash but without specifying any specific error now

[unleashed]

fix styling issue and 👍

[miguelsorianod]

commits have been squashed

[miguelsorianod]

bors r=@unleashed,@eguzki

[bors]

Build succeeded

• ci/circleci