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

Middleware not added to chain? #2

Closed
cmer opened this issue Sep 19, 2012 · 10 comments
Closed

Middleware not added to chain? #2

cmer opened this issue Sep 19, 2012 · 10 comments

Comments

@cmer
Copy link

cmer commented Sep 19, 2012

I added the gem to my Gemfile and I couldn't get UniqueJobs to work. I pry'ed my code and it looks like the middleware is not automatically inserted in the chain.

I tried adding it in an initializer as well as manually in a Pry session but to no avail. Oddly enough, the Client middleware is added. Any idea what might be going on? Here's some code:

[1] pry(#<NotificationPushWorker>)> Sidekiq.server_middleware
=> #<Sidekiq::Middleware::Chain:0x007fd675435e10
 @entries=
  [#<Sidekiq::Middleware::Entry:0x007fd675435d70
    @args=[],
    @klass=Sidekiq::Middleware::Server::Logging>,
   #<Sidekiq::Middleware::Entry:0x007fd675435cd0
    @args=[],
    @klass=Sidekiq::Middleware::Server::RetryJobs>,
   #<Sidekiq::Middleware::Entry:0x007fd675435c30
    @args=[],
    @klass=Sidekiq::Middleware::Server::ActiveRecord>,
   #<Sidekiq::Middleware::Entry:0x007fd675435b90
    @args=[],
    @klass=Sidekiq::Middleware::Server::Timeout>]>
[2] pry(#<NotificationPushWorker>)> Sidekiq.configure_server do |config|
[2] pry(#<NotificationPushWorker>)*   config.server_middleware do |chain|  
[2] pry(#<NotificationPushWorker>)*     require 'sidekiq-unique-jobs/middleware/server/unique_jobs'    
[2] pry(#<NotificationPushWorker>)*     chain.add SidekiqUniqueJobs::Middleware::Server::UniqueJobs    
[2] pry(#<NotificationPushWorker>)*   end    
[2] pry(#<NotificationPushWorker>)* end  
=> nil
[3] pry(#<NotificationPushWorker>)> 
[4] pry(#<NotificationPushWorker>)> Sidekiq.server_middleware
=> #<Sidekiq::Middleware::Chain:0x007fd675435e10
 @entries=
  [#<Sidekiq::Middleware::Entry:0x007fd675435d70
    @args=[],
    @klass=Sidekiq::Middleware::Server::Logging>,
   #<Sidekiq::Middleware::Entry:0x007fd675435cd0
    @args=[],
    @klass=Sidekiq::Middleware::Server::RetryJobs>,
   #<Sidekiq::Middleware::Entry:0x007fd675435c30
    @args=[],
    @klass=Sidekiq::Middleware::Server::ActiveRecord>,
   #<Sidekiq::Middleware::Entry:0x007fd675435b90
    @args=[],
    @klass=Sidekiq::Middleware::Server::Timeout>]>
[5] pry(#<NotificationPushWorker>)> Sidekiq.client_middleware
=> #<Sidekiq::Middleware::Chain:0x007fd698cc2628
 @entries=
  [#<Sidekiq::Middleware::Entry:0x007fd698cb6328
    @args=[],
    @klass=SidekiqUniqueJobs::Middleware::Client::UniqueJobs>]>

Thanks!

@cmer
Copy link
Author

cmer commented Oct 2, 2012

I updated to 2.3.2 and I still have the same problem... Any idea what might be wrong? It's kind of a show-stopper for me unfortunately.

Thanks!

@mhenrixon
Copy link
Owner

Real sorry you are having that problem still! Uniqueness should have been added automagically. Any chance you could share the project with us or if that isn't possible could you try and replicate the problem in a fresh repo?

@cmer
Copy link
Author

cmer commented Oct 2, 2012

Thanks for caring. I put together a small project that reproduces the problem.

Just run bundle and rake spec.

https://dl.dropbox.com/u/132165/unique-job-bug.zip

I also ran into another issue I haven't seen before, but I wonder if it's a Sidekiq but rather than a unique-jobs bug. I marked that test as "pending".

@mhenrixon
Copy link
Owner

It's obviously getting late, I started running tests instead of specs. I'll be back

@mperham
Copy link

mperham commented Oct 3, 2012

Are you sure that middleware is executed in the tests? I'm not sure it is.

@cmer
Copy link
Author

cmer commented Oct 3, 2012

FYI I just realized that in my spec file, I try to queue a whole activerecord object. The problem still persists when I replace the object with a simple string such as 'abc'.

NotificationPushWorker.perform_in(5.minutes, 'abc')

@mhenrixon
Copy link
Owner

Yup mike is right, to test this you would need to test against a real redis since the sidekiq/test doesn't take into consideration any middleware, it just pushes everything to jobs array without hesitation.

before {
  Sidekiq.redis = REDIS
  Sidekiq.redis {|c| c.flushdb }
}

it "does not store duplicate jobs" do
  3.times {
    Sidekiq::Client.push('class' => NotificationPushWorker, 'queue' => 'notification_push',  'args' => notification.id)
  }
  Sidekiq.redis { |c| c.llen('queue:notification_push') }.should eql 1
end

Really not ideal agreeably, I'll see if I can add some sort of hook for testing this in a client app.

@cmer
Copy link
Author

cmer commented Oct 3, 2012

Ah! So basically, you're saying that my tests fail but that in real life, it should just work?

I guess I'll test with the workaround for now. Thanks!

@davidcelis
Copy link

I can confirm via playing around on my own apps that the middleware does work and get executed. My jobs are unique, but I agree that not seeing it show up in the client middleware stack was confusing. I'm just glad this is working.

@mhenrixon
Copy link
Owner

Awesome :)

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

No branches or pull requests

4 participants