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

[Help request] Local variable outside of Disc::WorkerGroup was out of scope #12

Merged
merged 1 commit into from
Aug 5, 2015
Merged

Conversation

mzemel
Copy link
Contributor

@mzemel mzemel commented Aug 4, 2015

Hi Pote,

I'm not sure if this is more of help request or a pull request.

I had tried to start Disc with the Rails/ActiveJob integration aspect of the readme. The problem arose when I tried to run:

DISQUE_NODES=localhost:7711 QUEUES=urgent,default disc -r ./disc_init.rb

[Notice] Disc running in celluloid mode! Current DISC_CONCURRENCY is 25.
/Users/mzemel/.rbenv/versions/2.1.2/lib/ruby/gems/2.1.0/gems/disc-0.0.19/bin/disc:22:in `<class:WorkerGroup>': undefined local variable or method `concurrency' for Disc::WorkerGroup:Class (NameError)
    from /Users/mzemel/.rbenv/versions/2.1.2/lib/ruby/gems/2.1.0/gems/disc-0.0.19/bin/disc:20:in `<top (required)>'
    from /Users/mzemel/.rbenv/versions/2.1.2/bin/disc:23:in `load'
    from /Users/mzemel/.rbenv/versions/2.1.2/bin/disc:23:in `<main>'

In IRB, you cannot access a local variable defined out of the scope of a class, but you can reference a method. However, in this case even a method or instance variable does not share scope with the internals of the Disc::WorkerGroup class (I tried a little hacking on it). I assume this has to do with method_missing and details I do not fully grok.

require 'disc'
require 'clap'

Clap.run ARGV,
  "-r" => lambda { |file| require file }

if defined?(Celluloid)
  concurrency = Integer(ENV.fetch('DISC_CONCURRENCY', '25'))
  $stdout.puts(
    "[Notice] Disc running in celluloid mode! Current DISC_CONCURRENCY is\
 #{ concurrency }."
  )

  Disc::Worker.send(:include, Celluloid)

  if defined?(Celluloid::SupervisionGroup)
    # Deprecated as of Celluloid 0.17, but still supported via "backported mode"
    class Disc::WorkerGroup < Celluloid::SupervisionGroup
      pool Disc::Worker,
            size: concurrency,

Ex. 1 Failing code.

Using the code in the pull request, I was able to move past this error message. However, I do not see any enqueued jobs from Rails showing up in my running disc process.

If anything stands out immediately as having been a problem on another machine and there's an easy solution, let me know because I'd love to try out Disc/Disque. Please do not merge this PR as it has not fixed my issue.

@mzemel mzemel changed the title Local variable outside of Disc::WorkerGroup was out of scope [Help request] Local variable outside of Disc::WorkerGroup was out of scope Aug 4, 2015
@foca
Copy link
Collaborator

foca commented Aug 5, 2015

Oh, you're right about that being broken. Thanks for catching it.

I'm on my phone right now, but the PR is definitely mergeable! 😄

Would you mind opening an issue for your problem with Rails/ActiveJob so that it's not lost in the merged PR? Thanks! ❤️ ❤️ ❤️

foca added a commit that referenced this pull request Aug 5, 2015
Local variable outside of Disc::WorkerGroup was out of scope
@foca foca merged commit 393cc70 into pote:master Aug 5, 2015
@pote
Copy link
Owner

pote commented Aug 5, 2015

Just pushed v0.0.20 to RubyGems with your updates - they might not entirely solve the problem you're having, but they do avoid others, and versions are cheap.

Regarding not having jobs enqueued: try running disque monitor on a different terminal and enqueuing something, so we can make sure if anything is getting to Disque at all or we have a problem with the adapter, unfortunately as we've used Rails less and less over the last few years our testing of it might have been too basic and with the newest versions only.

Thanks a lot for trying Disc and open a PR/issue here, it's incredibly helpful to us :)

@mzemel
Copy link
Contributor Author

mzemel commented Aug 5, 2015

@pote You are correct: using disque monitor allows me to see the enqueued jobs:

1438791641.925413 [127.0.0.1:49646] "HELLO"
1438791641.926327 [127.0.0.1:49647] "ADDJOB" "urgent" "\x82\xa5class\xadDiscActiveJob\xa9arguments\x90" "100" "delay" ""
1438791641.927692 [127.0.0.1:49647] "ADDJOB" "urgent" "\x82\xa5class\xa7DiscJob\xa9arguments\x91\xafrandom_argument" "100" "delay" ""

NOTE: There are two jobs here: one from ActiveJob, and one directly on the service object, as you can see in the controller code at the bottom of this comment.

However, the process running disc does not show any output in both Rails and outside of rails. Here is my job, and while I'm assuming STDOUT will dump to the terminal, that may not be the case.

# disc_job.rb
require 'disc'
class DiscJob
  include Disc::Job
  disc queue: "urgent"

  def perform(*args)
    STDOUT.puts args.inspect
    process_id = Process.pid
    thread_id = Thread.current.object_id
    STDOUT.puts "process #{process_id}; thread #{thread_id}"
  end
end

With Rails, I invoke it through this controller, though I just require the disc_job.rb file in IRB when I want to test it outside of rails.

class DiscsController < ApplicationController
  def send_delayed_job
    STDOUT.puts "Through ActiveJob"
    DiscActiveJob.perform_later
    STDOUT.puts "Through Disc"
    DiscJob.enqueue("random_argument") # line I run in IRB
    render text: "OK"
  end
end

I'm happy to open something with ActiveJob, but because I'm having trouble invoking it manually I don't know that it's isolated to the ActiveJob adapter.

Thanks for all your hard work - I'm very excited to use Disc/Disque.

@pote
Copy link
Owner

pote commented Aug 5, 2015

Ahh, got it, v0.0.21 is pushed with the fix. The code that parsed which queues to listen to was broken, so instead of listening to the urgent and default queues it was listening to urgent,default, which doesn't have anything.

Test with the latest version and let me know if it's fixed. :)

@mzemel
Copy link
Contributor Author

mzemel commented Aug 5, 2015

@pote Thank you for pushing the fix! It works on my machine. Thanks for your help and communication. :)

@pote
Copy link
Owner

pote commented Aug 5, 2015

🎉 ❤️ ❤️ ❤️ ❤️ 🎉

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.

3 participants