search

imTigger / laravel-job-status

Add ability to track Job progress, status and result dispatched to Queue.
MIT License
407 stars 60 forks source link

Overhaul, new major release? #38

Closed crashkonijn closed 5 years ago

crashkonijn commented 5 years ago

Hi there,

This morning I came up with a better way of handling the improvement that I proposed in #37. While I was at it I also changed the following:

If you choose to accept this PR #36 & #37 can be closed.

Edit; if you enable travis for this project it will test new PR automatically: https://travis-ci.org/webparking/laravel-job-status

imTigger commented 5 years ago

Thank you so much, let me have a look first at this weekend :)

crashkonijn commented 5 years ago

@imTigger Do you know why this is here? (I can't seem to find it being used anywhere)

Also, would you mind if I'd change JobStatus::StatusX to a proper enum?

crashkonijn commented 5 years ago

On another note, I would like to add the 'exceptionOccurred' (or queuedForRetry or somehting similar) to show the difference between a job which has failed completely or which is still retrying.

crashkonijn commented 5 years ago

I changed the following:

imTigger commented 5 years ago

getAllowedStatuses() is unused internally, but used in my application only :) I would rather keep dependencies to minimal.

imTigger commented 5 years ago

I will merge this as 1.0 release soon. Will release after finishing some real-world testing on my application. Big thanks to @crashkonijn 👍

crashkonijn commented 5 years ago

Nice, thanks!

Let me know if you run into any problems.

Also, one thing that could use some more testing are the Trackable progress input and output methods. If someone could test those that would be great.

mrvnklm commented 5 years ago

I actually thought of creating a PR in order to allow jobs to have parents. For instance using ::dispatchChild() would dispatch the job and set a parent_id. What do you think @crashkonijn? Should I make a PR or would you implement it? Might just be a few lines

crashkonijn commented 5 years ago

@mrvnklm as of today I no longer work at Webparking, so personally I don't use this anymore. If there's something that needs fixing in the original PR I'm willing to make those changes, but unfortunately I don't have to time to develop new features.

mrvnklm commented 5 years ago

@crashkonijn no worries, I might make a PR once @imTigger does merge __DIR__ . '*/*../database/migrations/' is missing the highlighted slash in LaravelJobStatusServiceProvider.php

mrvnklm commented 5 years ago

@crashkonijn Am I doing something wrong? When I call $job = new TestJob(), $job->getJobStatusId() is null and if I try to $job->update([...]) the job dies without exception

imTigger commented 5 years ago

Sorry for the delay, it has been tough months in Hong Kong. Will tag a major release soon :)

https://twitter.com/hashtag/Eye4HK

crashkonijn commented 5 years ago

Understandable, I wish you all the best with that situation!

imTigger commented 4 years ago

I am working on a new project myself on Laravel 6 recently with 1.0-rc. Strange thing is the "getJobStatusId()" is actually called before "__sleep()", therefore prepareStatus() call still needed on each Job's constructors.

crashkonijn commented 4 years ago

Hey @imTigger!

Glad to see you survived the Honk Kong crisis so far!

Unfortunately I haven't been using Laravel for 7 months now, so I can't tell you why that's happening.

I do hope you figure it out and can merge this PR, I put a lot of time into it :)

imTigger commented 4 years ago

Thank you for your work, it works almost perfectly 👍 This PR has been merged in 1.0 release. I am adding more functionality and fixing CI build for 1.1 release.

Hope you all are staying well too :)