skroutz / rspecq

Distribute and run RSpec suites among parallel workers; for faster CI builds
https://rubygems.org/gems/rspecq
MIT License
161 stars 24 forks source link

Allow to pass redis options via ENV variable #52

Open 2rba opened 3 years ago

2rba commented 3 years ago

WHY

Secure redis connection might require additional connection params

WHAT

fragoulis commented 3 years ago

Hi and thank you for the time you took to make this. We have reviewed the PR at the time it was opened but we were overwhelmed with work (and still are).

Firstly, I agree with being able to support your use case. I have to say, however, I haven't seen this pattern of passing arbitrary options thourgh json to a cli. Can you provide of some examples of some other established cli tools that follow this pattern.

I would probably go with supporting a config file of some sort for more complex configuration. We also try to abide by some rules around the cli tool.

EDIT:

After seeing your addition to the README I updated the master with the current env variables. Thank you for that idea.

2rba commented 3 years ago

@jfragoulis Thank you for your comment. I don't like JSON options as well. There reason for it ssl_params might contain quite different options. From https://github.com/redis/redis-rb

:ssl_params => {
    :ca_file => "/path/to/ca.crt",
    :cert    => OpenSSL::X509::Certificate.new(File.read("client.crt")),
    :key     => OpenSSL::PKey::RSA.new(File.read("client.key"))
  }

And each environment might require different options. An attempt to define them all as command-line arguments might overload command-line options with noisy params.

I was thinking about what could be the other option, Could redis connection be an injectable config option? like Redisq.config.redis_connection this way if someone needs a hand-tailored Redis connection it can be injected somewhere in rails_helper.rb as

Redisq.config.redis_connection = Redis.new

What do you think?