ksassnowski/venture

Non queued jobs not being marked as finished

Closed this issue · 10 comments

notomato commented

Not sure if I am being stupid but none of the job/steps ever get marked as finished (no update to finished_at column). Is this done automatically? Do I need to do it manually or is anything required to be returned from the handle() method in the Jobs?

The workflow runs fine and all the jobs do complete, they are just not being marked as finished.

ksassnowski commented

That's odd. If the workflow runs correctly then it should definitely mark the jobs as finished as well since that happens on the same code path. Have you checked if the then method on the workflow gets called?

Can you maybe provide me with a repo where I can reproduce the error?

notomato commented

Sure - I just added dummy jobs and a dummy workflow - same again... will try with a vanilla laravel repo

notomato commented

A vanilla repo is updating the finished_at... will investigate more. Can close.

notomato commented

I thought I would re-open this because the docs don't make it quite clear what the expected behaviour should be.

Jobs that implement ShouldQueue will set the finish_at field. But jobs that are dispatched immediately and don't queue won't update the field.

I think I got confused by the docs because the jobs only show the use WorkflowStep;... what's the intent - should jobs that are dispatched immediately still work?

ksassnowski commented

Thanks for the information! Even jobs without the ShouldQueue interface should be marked as finished, so that sounds like a bug.

Off the top of my head I can't image why it wouldn't work without this interface. Especially since you mentioned that the workflow itself executes correctly.

I will have to look into this.

notomato commented

I guess that the JobProcessed and other listeners won't be triggered for non-queued jobs? I can just see the one place where they are marked as finished - coming from listening to that event.

ksassnowski commented

That was my initial thought as well, but that would mean that the entire workflow wouldn't work, which doesn't seem to be the case. I will try to have a look at this today.

Can you maybe upload your test repository so that I use it for debugging purposes?

ksassnowski commented

Turns out, your initial hunch was correct. If a job does not implement the ShouldQueue interface, it will bypass the Queue completely and as such none of the relevant events that Venture listens on ever get fired.

The problem goes further than simply not setting the finished_at timestamp, however. If these events don't get fired, none of the subsequent jobs will get executed. That means only the initial batch of jobs will be completed and then the workflow just sort of ... stops.

So what this means is that in order to use a job inside a workflow, they have to implement the ShouldQueue interface. If you still want to execute the job synchronously, you should set the jobs connection property to sync explicitly either in the job itself...

class Test1 implements ShouldQueue
{
    use WorkflowStep, Queueable;

    public function __construct()
    {
        $this->connection = 'sync';
    }

... or when creating the job instance.

return Workflow::define('My Workflow')
    ->addJob((new Test1())->onConnection('sync'));

I will update the documentation accordingly. For Venture 2 I will probably throw and exception when attempting to add a job to a workflow that doesn't implement the ShouldQueue interface.

Hope that clears it up. Thanks for the help!

notomato commented

Thanks! That's great. I do have another issue with the event firing twice now on queued jobs which I am trying to narrow down.