minion icon indicating copy to clipboard operation
minion copied to clipboard

Add debug message on job fail

Open pstray opened this issue 3 years ago • 6 comments

Summary

Just a simple patch to add a debug log on job fail.

Could possibly add some check if there already are on-fail events hooks, but not sure how to do that :)

Motivation

I find it really useful to actually see if a job fail in the terminal running ./app minion worker, and not have to inspect the web interface or add promises or hooks to jobs.

References

Was discussed on #mojo earlier this week

pstray avatar Oct 03 '20 12:10 pstray

Pretty sure i already said on IRC that this is not the right solution. The Mojo::Job class should not be tied to a Mojolicious app.

kraih avatar Oct 03 '20 12:10 kraih

The job id is missing from the log message also. And debug is probably not the right level.

kraih avatar Oct 03 '20 12:10 kraih

Not to mention the lack of tests.

kraih avatar Oct 03 '20 12:10 kraih

Ok, moved the message from Minion/Job.pm to Minion/Command/worker/minion.pm. Sounds like a much more appropriate place for it.

Still no tests though, mostly because I have no idea how to test it :)

Wether it should be debug or some other level can be discussed, but the other messages in that file are debug, and for me, I mostly use such things for debugging. If you really want to do anything with those error messages, I guess you should handle them in other ways in the app itself.

pstray avatar Oct 05 '20 17:10 pstray

That's a lot of code for what could have been a one-liner.

kraih avatar Oct 05 '20 17:10 kraih

Removed one of the functions, _dequeue, and fixed the indentation and some spacing to match the other code. Still, a bit more than a one-liner, but that wouldn't be very readable code :)

pstray avatar Oct 05 '20 18:10 pstray