jekyll / classifier-reborn

A general classifier module to allow Bayesian and other types of classifications. A fork of cardmagic/classifier.
https://jekyll.github.io/classifier-reborn/
GNU Lesser General Public License v2.1
553 stars 110 forks source link

Allow redis connection to be injected #188

Open allcentury opened 5 years ago

allcentury commented 5 years ago

It be easier to scale up our redis usage if we were able to pass around a redis connection to BayesRedisBackend rather than let that class hold it's own connection. In fact, I think everyone benefits from this because this would also allow folks to use other redis gems as long as it conformed to the interface.

The idea here might look like this:

class MyJob < ApplicationJob
  def perform
    ClassifierReborn::BayesRedisBackend.new(
      host: ENV["REDIS_HOST"],
      port: ENV["REDIS_PORT"].to_i,
      db: ENV["REDIS_DB_EVIDENCE_CLASSIFIER_DB"].to_i,
    )
    @trainer = ClassifierReborn::Bayes.new(backend: backend)
  end
end

# => now N concurrency = N connections

Instead:

class RedisPool
  def self.connections
    @c ||= ConnectionPool.new(size: 5, timeout: 5) do
      Redis.new(
        host: ENV["REDIS_HOST"],
        port: ENV["REDIS_PORT"].to_i,
        db: ENV["REDIS_DB_EVIDENCE_CLASSIFIER_DB"].to_i,
      )
    end
  end
end

class MyJob < ApplicationJob
  def perform
    RedisPool.connections.with_conn do |conn|
      backend = ClassifierReborn::BayesRedisBackend.new(client: conn)
      @trainer = ClassifierReborn::Bayes.new(backend: backend)
    end
  end
end

Would you be open to a PR for this?

Ch4s3 commented 5 years ago

Submit a PR and I'll take a look as soon as I can