Setup upgrade to Celluloid 0.17.2

brandonhilkert commented 9 years ago

@tarcieri Any chance you could offer some insight? I'm looking to get Sucker Punch on to 0.17.2. I'm getting the same errors any time I try to call .pool on a job with Celluloid included.

Here are the Travis failures and here is the full stack track locally if I try to call FakeJob.pool:

E, [2015-10-03T11:29:41.624797 #3640] ERROR -- : Actor crashed!
NoMethodError: undefined method `[]=' for nil:NilClass
    /Users/bhilkert/.rbenv/versions/2.2.0/lib/ruby/2.2.0/pp.rb:148:in `ensure in guard_inspect_key'
    /Users/bhilkert/.rbenv/versions/2.2.0/lib/ruby/2.2.0/pp.rb:148:in `guard_inspect_key'
    /Users/bhilkert/.rbenv/versions/2.2.0/lib/ruby/2.2.0/pp.rb:99:in `pp'
    /Users/bhilkert/.rbenv/versions/2.2.0/lib/ruby/2.2.0/pp.rb:12:in `pretty_inspect'
    /Users/bhilkert/.rbenv/versions/2.2.0/lib/ruby/gems/2.2.0/gems/celluloid-0.17.2/lib/celluloid/calls.rb:28:in `public_send'
    /Users/bhilkert/.rbenv/versions/2.2.0/lib/ruby/gems/2.2.0/gems/celluloid-0.17.2/lib/celluloid/calls.rb:28:in `dispatch'
    /Users/bhilkert/.rbenv/versions/2.2.0/lib/ruby/gems/2.2.0/gems/celluloid-0.17.2/lib/celluloid/call/sync.rb:16:in `dispatch'
    /Users/bhilkert/.rbenv/versions/2.2.0/lib/ruby/gems/2.2.0/gems/celluloid-0.17.2/lib/celluloid/cell.rb:50:in `block in dispatch'
    /Users/bhilkert/.rbenv/versions/2.2.0/lib/ruby/gems/2.2.0/gems/celluloid-0.17.2/lib/celluloid/cell.rb:76:in `block in task'
    /Users/bhilkert/.rbenv/versions/2.2.0/lib/ruby/gems/2.2.0/gems/celluloid-0.17.2/lib/celluloid/actor.rb:339:in `block in task'
    /Users/bhilkert/.rbenv/versions/2.2.0/lib/ruby/gems/2.2.0/gems/celluloid-0.17.2/lib/celluloid/task.rb:44:in `block in initialize'
    /Users/bhilkert/.rbenv/versions/2.2.0/lib/ruby/gems/2.2.0/gems/celluloid-0.17.2/lib/celluloid/task/fibered.rb:14:in `block in create'
=> #<#<Class:#<Celluloid::Proxy::Cell:0x007f84ac289c08>>:0x3fc256144e04>

I tried to dig in to public_send, but it wasn't clear what was going on.

pboling commented 9 years ago

@brandonhilkert I think @donaldpiret fixed an issue similar to the one you had above with []=: https://github.com/donaldpiret/sucker_punch/commit/41179c356da96878f4cfd3da14cba94796869119#diff-67ee592b349f1f51631f3d1a28b3720dL65

It is in his PR: https://github.com/brandonhilkert/sucker_punch/pull/126

brandonhilkert commented 9 years ago

@pboling Maybe I'm missing something, but you're pointing to a commit that removes registering the pool altogether. I don't see how Sucker Punch would enqueue to the same queue after that. It appears to me as though it would create a new queue for every job.

pboling commented 9 years ago

I'll not pretend to be familiar with Celluloid at all. I just noticed that he had removed this:

Celluloid::Actor[name] = pool

and found a workaround, and it looks like the removed code may have been removed due to the same error you are getting.

E, [2015-10-03T11:29:41.624797 #3640] ERROR -- : Actor crashed!
NoMethodError: undefined method `[]=' for nil:NilClass

But it is really just a guess. Sorry if I'm not helping. I am trying to build something vaguely similar to sucker punch right now, and reading your code for inspiration. I only have a few days on Celluloid thus far.

donaldpiret commented 9 years ago

@brandonhilkert if I'm not mistaken I'm actually just replacing the pool with a supervision container with a managed pool, which is what celluloid 0.17 seems to want us to do. I think the pool is still correctly named and tasks are still correctly assigned to it. Then again I have to admit I'm not super familiar with the Celluloid workings either so I might have no clue about what I'm doing :p

brandonhilkert / sucker_punch

Setup upgrade to Celluloid 0.17.2 #130