activejob-locking icon indicating copy to clipboard operation
activejob-locking copied to clipboard

Redis port and password aren't supported

Open bkroeker opened this issue 5 years ago • 5 comments

Hi there. I'm using the suo adapter on my project and I had to hack activejob-locking to pass through a port and password for the redis server to the underlying suo gem. I'd submit a pull request, but the activejob-locking gem's current methodology for distributed servers is kind of counter-intuitive for the redis case. The suo adapter, for example, just uses { host: options.hosts.first } and drops any other hosts in the array. Would I ideally pass in an array of ports and passwords which would each get truncated to the first element?

In any case, supporting ports and passwords, or entire connection strings, seems like a pretty basic feature, which I would like to see supported at some point.

Cheers. :)

bkroeker avatar May 21 '19 16:05 bkroeker

ActiveJob::Locking.options.hosts = ['localhost']

In the suo adapter, the code is:

class SuoRedis < Base
    def create_lock_manager
      mapped_options = {connection: {host: self.options.hosts.first},
                        stale_lock_expiration: self.options.lock_time,
                        acquisition_timeout: self.options.lock_acquire_time}

      Suo::Client::Redis.new(self.key, mapped_options)
    end`

Notice it takes the first host specified in options. So configure that value based on the suo documentation.

cfis avatar Sep 04 '19 06:09 cfis

@cfis Thanks, but I don't think you read my post. I'm trying to specify ports and passwords, and/or entire connection strings.

bkroeker avatar Sep 04 '19 13:09 bkroeker

Depending on the adapter, I think you can use the host string to specify the port and/or password.

cfis avatar Sep 09 '19 19:09 cfis

In this case, to set the full connection string, you'd need to drop the host hash. So:

mapped_options = { connection: "127.0.0.1:11211", ... }

Basically, with the current implementation, it is impossible to specify ports or passwords using the Suo adapter. The create_lock_manager method you've printed out above is the problem -- it needs to be opened up to be more flexible. I could submit a pull request to do so, but I wasn't sure what strategy to employ in opening that method up, since doing so would make it inconsistent from the other adapters.

Maybe it needs to do something like mapped_options.merge!(self.options.adapter_options) to overwrite the limitations you've hardcoded into the method, kind of like you're doing in the RedisSemaphore adapter?

IMO, you should get rid of options.hosts completely and just have options.adapter_options used universally across all adapters, relying on the developer to pass in a proper configuration for their redis gem-of-choice. I don't think the hardcoded configuration limitations buy you anything, from what I can see.

bkroeker avatar Sep 09 '19 21:09 bkroeker

Makes sense. Happy with your idea, can you code it up?

cfis avatar Oct 06 '19 19:10 cfis