Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Let's try this again... #149

Merged
merged 2 commits into from

3 participants

@sferik

Use Object#respond_to? to determine which MultiJson API to use.

@sferik sferik Let's try this again...
Use `Object#respond_to?` to determine which MultiJson API to use.
68c725e
@jc00ke
Collaborator

I'd much rather see us use a method like Sidekiq.decode all over the place & check the version in there. Not a fan of introducing those conditionals everywhere.

@mperham
Owner

I'm supposed to add this ugly hack because you don't want to alias dump to encode?

@sferik

@mperham Encode is aliased to dump, it just throws warnings. If you don't want warnings, you need to feature-detect.

@mperham
Owner

Yeah, I'm saying there's no reason for the warnings. Just support both pairs. dump/load and encode/decode are both perfectly reasonable names.

@sferik sferik Refactor to use Sidekiq.dump_json and Sidekiq.load_json
These methods perform MultiJson feature detection and can be removed
after this library's MultiJson dependency is upgraded to ~> 2.0.
5eb8d39
@sferik

I've cleaned up the patch, per @jc00ke's suggestion.

@mperham mperham merged commit c852d7e into mperham:master
@mperham
Owner

Much better, thanks.

@jc00ke
Collaborator

Yes, much better. Thanks @sferik!

@sferik

My pleasure.

When MultiJson 2.0 is released and you're ready to upgrade, you should be able to safely remove Sidekiq.[dump|load]_json and then run these commands:

git ls-files *.rb | xargs sed -i '' 's/Sidekiq.load_json/MultiJson.load/g'
git ls-files *.rb | xargs sed -i '' 's/Sidekiq.dump_json/MultiJson.dump/g'
@jc00ke
Collaborator

I would actually prefer to leave as is and only change what's inside Sidekiq.[dump|load]_json if necessary. No reason to create a greater surface area, and in case we ever want to change/update in the future it'll be easy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Apr 22, 2012
  1. @sferik

    Let's try this again...

    sferik authored
    Use `Object#respond_to?` to determine which MultiJson API to use.
  2. @sferik

    Refactor to use Sidekiq.dump_json and Sidekiq.load_json

    sferik authored
    These methods perform MultiJson feature detection and can be removed
    after this library's MultiJson dependency is upgraded to ~> 2.0.
This page is out of date. Refresh to see the latest.
View
18 lib/sidekiq.rb
@@ -82,4 +82,22 @@ def self.server_middleware
@server_chain
end
+ def self.load_json(string, options={})
+ # Can't reliably detect whether MultiJson responds to load, since it's
+ # a reserved word. Use adapter as a proxy for new features.
+ if MultiJson.respond_to?(:adapter)
+ MultiJson.load(string, options)
+ else
+ MultiJson.decode(string, options)
+ end
+ end
+
+ def self.dump_json(object, options={})
+ if MultiJson.respond_to?(:dump)
+ MultiJson.dump(object, options)
+ else
+ MultiJson.encode(object, options)
+ end
+ end
+
end
View
2  lib/sidekiq/client.rb
@@ -45,7 +45,7 @@ def self.push(item)
pushed = false
Sidekiq.client_middleware.invoke(worker_class, item, queue) do
- payload = MultiJson.encode(item)
+ payload = Sidekiq.dump_json(item)
Sidekiq.redis do |conn|
_, pushed = conn.multi do
conn.sadd('queues', queue)
View
2  lib/sidekiq/manager.rb
@@ -110,7 +110,7 @@ def assign(msg, queue)
processor = @ready.pop
@in_progress[processor.object_id] = [msg, queue]
@busy << processor
- processor.process!(MultiJson.decode(msg), queue)
+ processor.process!(Sidekiq.load_json(msg), queue)
end
end
end
View
4 lib/sidekiq/middleware/client/unique_jobs.rb
@@ -1,5 +1,5 @@
-require 'multi_json'
require 'digest'
+require 'multi_json'
module Sidekiq
module Middleware
@@ -10,7 +10,7 @@ class UniqueJobs
def call(worker_class, item, queue)
enabled = worker_class.get_sidekiq_options['unique']
if enabled
- payload_hash = Digest::MD5.hexdigest(MultiJson.encode(item))
+ payload_hash = Digest::MD5.hexdigest(Sidekiq.dump_json(item))
unique = false
Sidekiq.redis do |conn|
View
5 lib/sidekiq/middleware/server/failure_jobs.rb
@@ -1,3 +1,5 @@
+require 'multi_json'
+
module Sidekiq
module Middleware
module Server
@@ -14,8 +16,7 @@ def call(*args)
:worker => args[1]['class'],
:queue => args[2]
}
-
- Sidekiq.redis {|conn| conn.rpush(:failed, MultiJson.encode(data)) }
+ Sidekiq.redis {|conn| conn.rpush(:failed, Sidekiq.dump_json(data)) }
raise
end
end
View
4 lib/sidekiq/middleware/server/retry_jobs.rb
@@ -1,3 +1,5 @@
+require 'multi_json'
+
require 'sidekiq/retry'
module Sidekiq
@@ -44,7 +46,7 @@ def call(worker, msg, queue)
delay = DELAY.call(count)
logger.debug { "Failure! Retry #{count} in #{delay} seconds" }
retry_at = Time.now.to_f + delay
- payload = MultiJson.encode(msg)
+ payload = Sidekiq.dump_json(msg)
Sidekiq.redis do |conn|
conn.zadd('retry', retry_at.to_s, payload)
end
View
4 lib/sidekiq/middleware/server/unique_jobs.rb
@@ -1,3 +1,5 @@
+require 'multi_json'
+
module Sidekiq
module Middleware
module Server
@@ -5,7 +7,7 @@ class UniqueJobs
def call(*args)
yield
ensure
- json = MultiJson.encode(args[1])
+ json = Sidekiq.dump_json(args[1])
hash = Digest::MD5.hexdigest(json)
Sidekiq.redis {|conn| conn.del(hash) }
end
View
5 lib/sidekiq/processor.rb
@@ -1,4 +1,5 @@
require 'celluloid'
+require 'multi_json'
require 'sidekiq/util'
require 'sidekiq/middleware/server/active_record'
@@ -53,8 +54,8 @@ def stats(worker, msg, queue)
redis do |conn|
conn.multi do
conn.set("worker:#{self}:started", Time.now.to_s)
- conn.set("worker:#{self}", MultiJson.encode(:queue => queue, :payload => msg,
- :run_at => Time.now.strftime("%Y/%m/%d %H:%M:%S %Z")))
+ hash = {:queue => queue, :payload => msg, :run_at => Time.now.strftime("%Y/%m/%d %H:%M:%S %Z")}
+ conn.set("worker:#{self}", Sidekiq.dump_json(hash))
end
end
View
2  lib/sidekiq/retry.rb
@@ -46,7 +46,7 @@ def poll
messages.each do |message|
logger.debug { "Retrying #{message}" }
- msg = MultiJson.decode(message)
+ msg = Sidekiq.load_json(message)
conn.rpush("queue:#{msg['queue']}", message)
end
end
View
10 lib/sidekiq/web.rb
@@ -52,7 +52,7 @@ def workers
Sidekiq.redis do |conn|
conn.smembers('workers').map do |w|
msg = conn.get("worker:#{w}")
- msg = MultiJson.decode(msg) if msg
+ msg = Sidekiq.load_json(msg) if msg
[w, msg]
end.sort { |x| x[1] ? -1 : 1 }
end
@@ -74,7 +74,7 @@ def retry_count
def retries
Sidekiq.redis do |conn|
results = conn.zrange('retry', 0, 25, :withscores => true)
- results.each_slice(2).map { |msg, score| [MultiJson.decode(msg), Float(score)] }
+ results.each_slice(2).map { |msg, score| [Sidekiq.load_json(msg), Float(score)] }
end
end
@@ -89,7 +89,7 @@ def queues
def retries_with_score(score)
Sidekiq.redis do |conn|
results = conn.zrangebyscore('retry', score, score)
- results.map { |msg| MultiJson.decode(msg) }
+ results.map { |msg| Sidekiq.load_json(msg) }
end
end
@@ -124,7 +124,7 @@ def relative_time(time)
get "/queues/:name" do
halt 404 unless params[:name]
@name = params[:name]
- @messages = Sidekiq.redis {|conn| conn.lrange("queue:#{@name}", 0, 10) }.map { |str| MultiJson.decode(str) }
+ @messages = Sidekiq.redis {|conn| conn.lrange("queue:#{@name}", 0, 10) }.map { |str| Sidekiq.load_json(str) }
slim :queue
end
@@ -142,7 +142,7 @@ def relative_time(time)
results = conn.zrangebyscore('retry', score, score)
conn.zremrangebyscore('retry', score, score)
results.map do |message|
- msg = MultiJson.decode(message)
+ msg = Sidekiq.load_json(message)
conn.rpush("queue:#{msg['queue']}", message)
end
end
View
2  sidekiq.gemspec
@@ -17,7 +17,7 @@ Gem::Specification.new do |gem|
gem.add_dependency 'redis-namespace'
gem.add_dependency 'connection_pool', '~> 0.9.0'
gem.add_dependency 'celluloid', '~> 0.10.0'
- gem.add_dependency 'multi_json', '>= 1.0', '< 1.3'
+ gem.add_dependency 'multi_json', '~> 1.0'
gem.add_development_dependency 'minitest'
gem.add_development_dependency 'sinatra'
gem.add_development_dependency 'slim'
View
3  test/test_retry.rb
@@ -1,4 +1,5 @@
require 'helper'
+require 'multi_json'
require 'sidekiq/retry'
require 'sidekiq/middleware/server/retry_jobs'
@@ -81,7 +82,7 @@ def @redis.with; yield self; end
end
it 'should poll like a bad mother...SHUT YO MOUTH' do
- fake_msg = MultiJson.encode({ 'class' => 'Bob', 'args' => [1,2], 'queue' => 'someq' })
+ fake_msg = Sidekiq.dump_json({ 'class' => 'Bob', 'args' => [1,2], 'queue' => 'someq' })
@redis.expect :multi, [[fake_msg], 1], []
@redis.expect :rpush, 1, ['queue:someq', fake_msg]
Something went wrong with that request. Please try again.