search

rails / activejob

Declare job classes that can be run by a variety of queueing backends
743 stars 47 forks source link

Persist JOB-ID #78

Closed cristianbica closed 10 years ago

cristianbica commented 10 years ago

When a new job is scheduled it will generate a JOB-ID and will pass this to the adapter along with the arguments. Before execution the JOB-ID is extracted and assigned to the job instance to be used in logging. It very useful for debugging (for example if you have retries you can easily grep the log file after the JOB-ID). I started this by working on using the JOB-ID from the adapter (where available) but it seems that only sidekiq can do that so I've decided not to use the JOB-ID from the provider for now.

Also a few small additions:

cristianbica commented 10 years ago

Fixed all the comments

cristianbica commented 10 years ago

removed the = nil and rebased /cc @dhh

dhh commented 10 years ago

There are two CI errors on Qu and Sucker Punch from this, says Travis CI.

cristianbica commented 10 years ago

Qu is rescueing exceptions from the job and logs them but they don't re-raise. Sucker_punch logs but it seems they are raising the exception. Errors running test_qu I think we can merge this and I'll work on a solution to make :qu raise

dhh commented 10 years ago

I’m not keen on breaking the build. Then no other PRs can rely on it for failures.

On Jun 12, 2014, at 1:19 PM, Cristian Bica notifications@github.com wrote:

Qu is rescueing exceptions from the job and logs them but they don't re-raise. Sucker_punch logs but it seems they are raising the exception. Errors running test_qu I think we can merge this and I'll work on a solution to make :qu raise

— Reply to this email directly or view it on GitHub.

cristianbica commented 10 years ago

Ok. Now that I look better it seems that it's an issue with this PR + qu as it passed on the qu_adapter PR. Looking into it

cristianbica commented 10 years ago

I've changed the rescue test case from JobClass.new.execute("args") to JobClass.enqueue("args")

cristianbica commented 10 years ago

Reverted the changes to the test/cases/rescue_test.rb. It passes now. It's normal for the adapter the rescue exceptions so their worker won't crash. The issue with qu is they shouldn't rescue on inline mode.