socialpandas / sidekiq-superworker

Directed acyclic graphs of Sidekiq jobs
MIT License
438 stars 34 forks source link

Undefined method `MyWorker` for MyModule:Module #13

Closed cheshire137 closed 10 years ago

cheshire137 commented 10 years ago

I have a collection of workers in a module, MyModule. I'm trying to make a superworker that will execute these workers in serial:

Superworker.create('MyModule::Superworker', :param1, :param2) do
  MyModule::Worker1 :param1, :param2
  MyModule::Worker2 :param1, :param2
  MyModule::Worker3 :param1, :param2
  MyModule::Worker4 :param1, :param2
  MyModule::Worker5 :param1, :param2
end

My workers are defined in app/workers/my_module like so:

module MyModule
  class Worker1
    include Sidekiq::Worker
    sidekiq_options queue: 'my_queue', backtrace: true, retry: false

    def perform param1, param2
      # ...
    end
  end
end

When I try to access MyModule::Superworker, which is defined in app/workers/my_module/superworker.rb, I get the following error in a Rails console:

>> MyModule::Superworker
NoMethodError: undefined method `Worker1' for MyModule:Module
  from /Users/me/code/my-project/app/workers/my_module/superworker.rb:2:in `block in <top (required)>'
  from /Users/me/.rvm/gems/ruby-2.0.0-p353/gems/sidekiq-superworker-0.1.4/lib/sidekiq/superworker/dsl_parser.rb:14:in `instance_eval'
  from /Users/me/.rvm/gems/ruby-2.0.0-p353/gems/sidekiq-superworker-0.1.4/lib/sidekiq/superworker/dsl_parser.rb:14:in `block in block_to_nested_hash'

Am I doing something wrong? Should sidekiq-superworker work with workers that are in a module? I can access the individual worker classes fine:

>> MyModule::Worker1
=> MyModule::Worker1
tombenner commented 10 years ago

Superworker classes are created using Object.const_set, and the current implementation doesn't support the definition of a worker within a module. However, this can easily be fixed.

If you'd like to add that support by modifying worker.rb (and ideally adding a test as well) and then submit a PR, that'd be great! I'm not sure how soon I'd have time to do it myself.

Thanks for catching this!

cheshire137 commented 10 years ago

Looking into it!— Sent from my iPad

On Wed, Mar 5, 2014 at 6:38 PM, Tom Benner notifications@github.com wrote:

Superworker classes are created using Object.const_set, and the current implementation doesn't support the definition of a worker within a module. However, this can easily be fixed. If you'd like to add that support by modifying worker.rb (and ideally adding a test as well) and then submit a PR, that'd be great! I'm not sure how soon I'd have time to do it myself.

Thanks for catching this!

Reply to this email directly or view it on GitHub: https://github.com/socialpandas/sidekiq-superworker/issues/13#issuecomment-36808604

cheshire137 commented 10 years ago

I think it's more complicated than changing the Object.const_set use. Before I even get to that part, it wigs out in DSLParser on line 14: @dsl_evaluator.instance_eval(&block). It says undefined method 'Worker10' for TestModule:Module, even though just before that part I can inspect TestModule::Worker10 and it's there.

tombenner commented 10 years ago

It looks like the issue you're seeing with DSLParser is related to the perform_async tests interfering with your tests, so I've isolated those and their accompanying dependencies in a new spec (in 7c46e5dfd168bee0a96f10610c313c66de03cd77). They could still use some cleanup, though.

This bug is trivial enough that I've gone ahead and fixed it (in 7d6fe6dcf7f5490c8c1bc87b6d8cb553aa32aa57). Let me know if master resolves this for you, and I'll go ahead and release it. Thanks again!

cheshire137 commented 10 years ago

I'm trying out the gem from master in Github. All my workers are in a module, while the superworker is outside the module. I now get the following error trying to reference my superworker:

>> MySuperworker
NoMethodError: undefined method `Worker1' for #<Module:0x007fc7f01e3a10>
  from /Users/me/my-project/app/workers/my_superworker.rb:2:in `block in <top (required)>'
  from /Users/me/.rvm/gems/ruby-2.0.0-p353/bundler/gems/sidekiq-superworker-7d6fe6dcf7f5/lib/sidekiq/superworker/dsl_parser.rb:14:in `instance_eval'
  from /Users/me/.rvm/gems/ruby-2.0.0-p353/bundler/gems/sidekiq-superworker-7d6fe6dcf7f5/lib/sidekiq/superworker/dsl_parser.rb:14:in `block in block_to_nested_hash'

This is slightly different from before, because it is looking for Worker1 in some anonymous module, it looks like, rather than MyModule.

I'll try putting the superworker in the module, too, see if that helps.

tombenner commented 10 years ago

Could you replace MyModule::Worker1 with MyModule__Worker1 and try again, with the new master?

It turns out that supporting MyModule::Worker1 would increase the complexity of the DSL parsing significantly (AFAICT), so the double underscore substitution may need to suffice for now (added in a9ad53fb09e). If you (or anyone else who sees this) would like to add support for the double colon, absolutely feel free to do so.

tombenner commented 10 years ago

This should be resolved now, but if anyone sees any issues, feel free to reopen.

millidavids commented 10 years ago

I am having this same problem. Seems like I still cannot use workers existing within a module.