Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support variadic exists and exists? #171

Merged
merged 1 commit into from Aug 19, 2020

Conversation

Tolsto
Copy link
Contributor

@Tolsto Tolsto commented Jun 22, 2020

redis-rb 4.2.0 made exists variadic and introduced exists?

redis-rb 4.2.0 made `exists` variadic and introduced `exists?`
@Tolsto
Copy link
Contributor Author

Tolsto commented Jun 22, 2020

Missed #169 but the tests could be merged.

@newellista
Copy link

Any chance we can get this merged and a new version released soon?

@mperham mperham mentioned this pull request Jul 6, 2020
@Tolsto
Copy link
Contributor Author

Tolsto commented Jul 7, 2020

Ping @rafaelfranca

@kriom
Copy link

kriom commented Jul 16, 2020

Ok for exists?

Passing 'exists?' command to redis as is; blind passthrough has been deprecated and will be removed in redis-namespace 2.0 (at ~/.rvm/gems/ruby-2.6.6/gems/sidekiq-6.1.0/lib/sidekiq/launcher.rb:161:in `block (2 levels) in ❤')

But what about: unlink I got this output when I run sidekiq

Passing 'unlink' command to redis as is; blind passthrough has been deprecated and will be removed in redis-namespace 2.0 (at ~/.rvm/gems/ruby-2.6.6/gems/sidekiq-6.1.0/lib/sidekiq/launcher.rb:148:in `block (2 levels) in ❤')

Edit:
OK, these messages disappear if I stop using the redis namespaces. ✌️😊

@QuantumWaver
Copy link

Any idea on the ETA for this?

@bf4
Copy link

bf4 commented Aug 7, 2020

Seems this gem isn't being actively maintained anymore :(

@bf4
Copy link

bf4 commented Aug 8, 2020

fwiw, this is my patch in a Rails initializer

require 'redis/namespace'
if Redis::Namespace::NAMESPACED_COMMANDS.key?("exists?")
  raise "Redis::Namespace now supports variadac exists, yay!"
else
  # https://github.com/resque/redis-namespace/pull/171/files
  Redis::Namespace::NAMESPACED_COMMANDS["exists?"] = [ :all ]
  Redis::Namespace::COMMANDS["exists?"] = [ :all ]
end

@ArturT
Copy link

ArturT commented Aug 8, 2020

@bf4 Does this workaround work for you?

I added

# config/initializers/0_redis_namespace.rb
require 'redis/namespace'
if Redis::Namespace::NAMESPACED_COMMANDS.key?("exists?")
  raise "Redis::Namespace now supports variadac exists, yay!"
else
  # https://github.com/resque/redis-namespace/pull/171/files
  Redis::Namespace::NAMESPACED_COMMANDS["exists?"] = [ :all ]
  Redis::Namespace::COMMANDS["exists?"] = [ :all ]
end

This is how I use Redis::Namespace

# config/initializers/redis.rb
redis_connection = Redis.new(url: Secret.sidekiq_redis_url, timeout: 5)

namespace = "#{Secret.app_namespace}:#{Rails.env}:web"
REDIS_WEB = Redis::Namespace.new(namespace, redis: redis_connection)

when I open rails console I get error

2.7.1 :001 > REDIS_WEB.exists?('fake')
Traceback (most recent call last):
        1: from (irb):1
SystemStackError (stack level too deep)

And when starting sidekiq then I get stack level too deep as well :/

...
/Users/artur/.rvm/gems/ruby-2.7.1/gems/redis-namespace-1.7.0/lib/redis/namespace.rb:357:in `method_missing'
/Users/artur/.rvm/gems/ruby-2.7.1/gems/redis-namespace-1.7.0/lib/redis/namespace.rb:357:in `method_missing'
/Users/artur/.rvm/gems/ruby-2.7.1/gems/redis-namespace-1.7.0/lib/redis/namespace.rb:357:in `method_missing'
/Users/artur/.rvm/gems/ruby-2.7.1/gems/redis-namespace-1.7.0/lib/redis/namespace.rb:357:in `method_missing'
/Users/artur/.rvm/gems/ruby-2.7.1/gems/redis-namespace-1.7.0/lib/redis/namespace.rb:357:in `method_missing'
/Users/artur/.rvm/gems/ruby-2.7.1/gems/redis-namespace-1.7.0/lib/redis/namespace.rb:357:in `method_missing'
/Users/artur/.rvm/gems/ruby-2.7.1/gems/redis-namespace-1.7.0/lib/redis/namespace.rb:357:in `method_missing'
/Users/artur/.rvm/gems/ruby-2.7.1/gems/redis-namespace-1.7.0/lib/redis/namespace.rb:357:in `method_missing'
/Users/artur/.rvm/gems/ruby-2.7.1/gems/redis-namespace-1.7.0/lib/redis/namespace.rb:357:in `method_missing'
/Users/artur/.rvm/gems/ruby-2.7.1/gems/redis-namespace-1.7.0/lib/redis/namespace.rb:357:in `method_missing'
/Users/artur/.rvm/gems/ruby-2.7.1/gems/redis-namespace-1.7.0/lib/redis/namespace.rb:357:in `method_missing'
/Users/artur/.rvm/gems/ruby-2.7.1/gems/redis-namespace-1.7.0/lib/redis/namespace.rb:357:in `method_missing'
/Users/artur/.rvm/gems/ruby-2.7.1/gems/sidekiq-6.1.0/lib/sidekiq/launcher.rb:161:in `block (2 levels) in ❤'
/Users/artur/.rvm/gems/ruby-2.7.1/gems/redis-namespace-1.7.0/lib/redis/namespace.rb:508:in `block in namespaced_block'
/Users/artur/.rvm/gems/ruby-2.7.1/gems/redis-4.2.1/lib/redis.rb:2491:in `block in multi'
/Users/artur/.rvm/gems/ruby-2.7.1/gems/redis-4.2.1/lib/redis.rb:69:in `block in synchronize'
/Users/artur/.rvm/rubies/ruby-2.7.1/lib/ruby/2.7.0/monitor.rb:202:in `synchronize'
/Users/artur/.rvm/rubies/ruby-2.7.1/lib/ruby/2.7.0/monitor.rb:202:in `mon_synchronize'
/Users/artur/.rvm/gems/ruby-2.7.1/gems/redis-4.2.1/lib/redis.rb:69:in `synchronize'
/Users/artur/.rvm/gems/ruby-2.7.1/gems/redis-4.2.1/lib/redis.rb:2484:in `multi'
/Users/artur/.rvm/gems/ruby-2.7.1/gems/redis-namespace-1.7.0/lib/redis/namespace.rb:505:in `namespaced_block'
/Users/artur/.rvm/gems/ruby-2.7.1/gems/redis-namespace-1.7.0/lib/redis/namespace.rb:293:in `multi'
/Users/artur/.rvm/gems/ruby-2.7.1/gems/sidekiq-6.1.0/lib/sidekiq/launcher.rb:159:in `block in ❤'
/Users/artur/.rvm/gems/ruby-2.7.1/gems/sidekiq-6.1.0/lib/sidekiq.rb:98:in `block in redis'
/Users/artur/.rvm/gems/ruby-2.7.1/gems/connection_pool-2.2.3/lib/connection_pool.rb:63:in `block (2 levels) in with'
/Users/artur/.rvm/gems/ruby-2.7.1/gems/connection_pool-2.2.3/lib/connection_pool.rb:62:in `handle_interrupt'
/Users/artur/.rvm/gems/ruby-2.7.1/gems/connection_pool-2.2.3/lib/connection_pool.rb:62:in `block in with'
/Users/artur/.rvm/gems/ruby-2.7.1/gems/connection_pool-2.2.3/lib/connection_pool.rb:59:in `handle_interrupt'
/Users/artur/.rvm/gems/ruby-2.7.1/gems/connection_pool-2.2.3/lib/connection_pool.rb:59:in `with'
/Users/artur/.rvm/gems/ruby-2.7.1/gems/sidekiq-6.1.0/lib/sidekiq.rb:95:in `redis'
/Users/artur/.rvm/gems/ruby-2.7.1/gems/sidekiq-6.1.0/lib/sidekiq/launcher.rb:158:in `❤'
/Users/artur/.rvm/gems/ruby-2.7.1/gems/sidekiq-6.1.0/lib/sidekiq/launcher.rb:97:in `heartbeat'
/Users/artur/.rvm/gems/ruby-2.7.1/gems/sidekiq-6.1.0/lib/sidekiq/launcher.rb:74:in `block in start_heartbeat'
/Users/artur/.rvm/gems/ruby-2.7.1/gems/sidekiq-6.1.0/lib/sidekiq/launcher.rb:73:in `loop'
/Users/artur/.rvm/gems/ruby-2.7.1/gems/sidekiq-6.1.0/lib/sidekiq/launcher.rb:73:in `start_heartbeat'
/Users/artur/.rvm/gems/ruby-2.7.1/gems/sidekiq-6.1.0/lib/sidekiq/util.rb:15:in `watchdog'
/Users/artur/.rvm/gems/ruby-2.7.1/gems/sidekiq-6.1.0/lib/sidekiq/util.rb:24:in `block in safe_thread'
#<Thread:0x00007fabe4f1b188@heartbeat /Users/artur/.rvm/gems/ruby-2.7.1/gems/sidekiq-6.1.0/lib/sidekiq/util.rb:22 run> terminated with exception (report_on_exception is true):
Traceback (most recent call last):
	8740: from /Users/artur/.rvm/gems/ruby-2.7.1/gems/sidekiq-6.1.0/lib/sidekiq/util.rb:24:in `block in safe_thread'
	8739: from /Users/artur/.rvm/gems/ruby-2.7.1/gems/sidekiq-6.1.0/lib/sidekiq/util.rb:15:in `watchdog'
	8738: from /Users/artur/.rvm/gems/ruby-2.7.1/gems/sidekiq-6.1.0/lib/sidekiq/launcher.rb:73:in `start_heartbeat'
	8737: from /Users/artur/.rvm/gems/ruby-2.7.1/gems/sidekiq-6.1.0/lib/sidekiq/launcher.rb:73:in `loop'
	8736: from /Users/artur/.rvm/gems/ruby-2.7.1/gems/sidekiq-6.1.0/lib/sidekiq/launcher.rb:74:in `block in start_heartbeat'
	8735: from /Users/artur/.rvm/gems/ruby-2.7.1/gems/sidekiq-6.1.0/lib/sidekiq/launcher.rb:97:in `heartbeat'
	8734: from /Users/artur/.rvm/gems/ruby-2.7.1/gems/sidekiq-6.1.0/lib/sidekiq/launcher.rb:158:in `❤'
	8733: from /Users/artur/.rvm/gems/ruby-2.7.1/gems/sidekiq-6.1.0/lib/sidekiq.rb:95:in `redis'
	 ... 8728 levels...
	   4: from /Users/artur/.rvm/gems/ruby-2.7.1/gems/redis-namespace-1.7.0/lib/redis/namespace.rb:357:in `method_missing'
	   3: from /Users/artur/.rvm/gems/ruby-2.7.1/gems/redis-namespace-1.7.0/lib/redis/namespace.rb:357:in `method_missing'
	   2: from /Users/artur/.rvm/gems/ruby-2.7.1/gems/redis-namespace-1.7.0/lib/redis/namespace.rb:357:in `method_missing'
	   1: from /Users/artur/.rvm/gems/ruby-2.7.1/gems/redis-namespace-1.7.0/lib/redis/namespace.rb:357:in `method_missing'
/Users/artur/.rvm/gems/ruby-2.7.1/gems/redis-namespace-1.7.0/lib/redis/namespace.rb:357:in `method_missing': stack level too deep (SystemStackError)
/Users/artur/.rvm/gems/ruby-2.7.1/gems/redis-namespace-1.7.0/lib/redis/namespace.rb:469: warning: Using the last argument as keyword parameters is deprecated; maybe ** should be added to the call
/Users/artur/.rvm/gems/ruby-2.7.1/gems/redis-4.2.1/lib/redis.rb:1961: warning: The called method `zrangebyscore' is defined here

I tried to use code from this PR directly in my app and it works fine

# Gemfile
gem 'redis-namespace', github: 'Tolsto/redis-namespace', branch: 'feature/new-exists-api'

@al
Copy link

al commented Aug 8, 2020

@ArturT Redis::Namespace::COMMANDS is processed when the Namespace class is loaded, so the patch above comes too late. You'll need to do a little more to make it work. Try something like:

class Redis::Namespace
  MY_HACKY_COMMANDS = {
    'exists' => [:all],
    'exists?' => [:all]
  }
  NAMESPACED_COMMANDS.reverse_merge! MY_HACKY_COMMANDS
  COMMANDS.reverse_merge! MY_HACKY_COMMANDS

  MY_HACKY_COMMANDS.keys.each do |command|
    next if method_defined?(command)

    define_method(command) do |*args, &block|
      call_with_namespace(command, *args, &block)
    end
  end
end

@ArturT
Copy link

ArturT commented Aug 8, 2020

@al Thank you! It works.

I added an exception when a new redis-namespace gem version will be released.

# config/initializers/redis.rb
# start of workaround
# https://github.com/resque/redis-namespace/pull/171#issuecomment-670917998
if Redis::Namespace::NAMESPACED_COMMANDS.key?("exists?")
  raise "Redis::Namespace now supports variadac exists, you can remove below workaround!"
end

class Redis::Namespace
  MY_HACKY_COMMANDS = {
    'exists' => [:all],
    'exists?' => [:all]
  }
  NAMESPACED_COMMANDS.reverse_merge!(MY_HACKY_COMMANDS)
  COMMANDS.reverse_merge!(MY_HACKY_COMMANDS)

  MY_HACKY_COMMANDS.keys.each do |command|
    next if method_defined?(command)

    define_method(command) do |*args, &block|
      call_with_namespace(command, *args, &block)
    end
  end
end
# end of workaround

@casperisfine
Copy link
Contributor

#169 was merged, this can be closed now.

@Krisztian-Molnar
Copy link

Can someone grant similar workaround for unlink as we have here for exists? command?

@PatrickTulskie
Copy link
Contributor

Can someone grant similar workaround for unlink as we have here for exists? command?

Not sure I follow. I don't see a unlink? in the redis gem.

@Krisztian-Molnar
Copy link

I'm talking about the second warning in this comment while running Sidekiq, I got the same:
#171 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet