Introduce Resque_Job_PID so a process PID can be obtained. #334
Conversation
mateusz
commented
Mar 30, 2017
•
mateusz
commented
HOWITWORKS.md
Outdated
| @@ -101,47 +101,50 @@ How do the workers process the queues? | |||
| 8. `Resque_Job->fail()` returns control to the worker (still in | |||
| `Resque_Worker::work()`) without a value | |||
| * Job | |||
| 1. The job calls `Resque_Worker->perform()` with the `Resque_Job` as its | |||
| 1. `Resque_Job_PID` is created, registering the PID of the actual process | |||
There was a problem hiding this comment.
Check your indentation. I think this file uses spaces instead of tabs.
HOWITWORKS.md
Outdated
| `COMPLETE`, then returns control, with no value, to the worker (again | ||
| still in `Resque_Worker::work()`) | ||
| 19. `Resque_Worker::work()` calls `exit(0)` to terminate the job process | ||
| 20. `Resque_Job_PID()` is removed, the forked process will terminate soon |
There was a problem hiding this comment.
Indentation again here.
README.md
Outdated
| @@ -193,6 +193,20 @@ or failed, and are then automatically expired. A status can also | |||
| forcefully be expired by calling the `stop()` method on a status | |||
| class. | |||
|
|
|||
| ### Obtaining woker process PID ### | |||
There was a problem hiding this comment.
Typo, plus this should probably say "job PID" ("process" is redundant).
lib/Resque/Job/PID.php
Outdated
| * | ||
| * @return int PID of the process doing the job (on non-forking OS, PID of the worker, otherwise forked PID). | ||
| */ | ||
| public function get() |
There was a problem hiding this comment.
Since this class doesn't let you do anything with the PID other than retrieve it, I'd just make this static, drop __construct() and __toString(), and pass the ID as an argument.
mateusz
force-pushed
the
pid-monitor
branch
2 times, most recently
from
814d976
to
15e2aa4
Compare
|
Thanks for prompt review - all fixed, good to review again. |
lib/Resque/Job/PID.php
Outdated
|
|
||
| /** | ||
| * Remove the PID tracker for the job. | ||
| */ |
There was a problem hiding this comment.
Should probably update the DocBlock, too. ;-)
|
Ha, missed that. Thanks for catching :-) How about now? |
|
It's an interesting feature for sure. Everything looks good from here, though it'd be nice to have some way to detect whether the PID belongs to the worker itself or not, and modify the return value accordingly... All said, 👍 |
|
Pray, what happens next? :-) |
|
Now we wait for @chrisboulton to have enough time free to properly review the PR himself, give any feedback he might have (if any), and decide whether to merge. This is the part that requires patience, I'm afraid. |
|
Allright. /pulls out his cigar, cognac, and settles on the armchair by the fireplace. |
|
Hi! Thanks for the contribution and working through the things @danhunsaker mentioned. To be honest, this seems to fit a very specific use case, and would be the kind of thing I'd suggest is something that's implemented as a plugin using the appropriate hooks (if there aren't the right ones there, lets fix that), rather than something that should be supported in core functionality? |
