Pull in old PRs from Chris's repo #7
Conversation
…en logging exceptions
this facilitates monitoring of each worker process
…ow semver installs for development versions
…methods `method_exists` will return true even if an object has a protected method; calling it from Resque will then fail. `is_callable` only returns true when the method both exists and can be called externally.
Chris's fork is outdated and unnecessary.
Allow those to fail, as we expect them to until we upgrade PHPUnit to one compatible with 7.2+. Also allow HHVM to fail, because we're going to be deprecating it soon anyway. Closes chrisboulton/php-resque#358
| @@ -217,7 +257,14 @@ public function work($interval = Resque::DEFAULT_INTERVAL, $blocking = false) | |||
| $this->logger->log(Psr\Log\LogLevel::INFO, $status); | |||
|
|
|||
| // Wait until the child process finishes before continuing | |||
| pcntl_wait($status); | |||
| while (pcntl_wait($status, WNOHANG) === 0) { | |||
There was a problem hiding this comment.
I was just about to start a fork to fix this issue as well!
However my fix was going to be registering the SIGTERM and SIGINT signals in registerSigHandlers with the restart_syscalls parameter set to false, allowing the handlers to be triggered.
Having said that, my knowledge of PHP and forking is quite limited and the documentation on pcntl_signal and the restart_syscalls parameter is slim. So changing pcntl_wait might actually be preferred over my fix. If not, let me know and I can create a PR for it.
There was a problem hiding this comment.
I'd say pull this branch down and give it a test. See if the current fix is sufficient. If not, toss us a PR with your changes as well, and we'll add them, too!
There was a problem hiding this comment.
I have done so and can confirm this change is working as expected, so no need for additional changes!
|
So what is the plan for having these reviewed? Can't wait for a new release ;-) |
|
Just waiting for somebody on the PHP Resque team (@resque/php) who isn't myself to submit a review. Can't review my own PRs! (GitHub literally doesn't allow it...) Once that's done, merging will be pretty quick! |
|
In the absence of reviews from the @resque/php team, I'm overriding the requirement, as a repo admin, for a review, and merging this now. It will go to the |
|
Deployed this two days ago where it would have little impact if broken. It has been running at least a job per minute, not a single problem so far 👍 On different note, when I follow the link in the @resque/php mentions, I get a 404. Maybe this is related to nobody responding to the call for review? |
|
You mean the link to the team itself? Or something else? |
The former, when referencing the team in your previous comments here. For example:
@resque/php links to https://github.com/orgs/resque/teams/php, but that's a 404 for me. So I was thinking they might not have received any notification about your request to review, because something is wrong with the team. Might be completely wrong though :) I cannot seem to reproduce the reference in a comment either, so I don't know how it works. |
|
Ah. Must mean the team is private. Interesting they've opted to 404 those rather than 403, but ¯\_(ツ)_/¯ I guess. |
This PR consists entirely of changes submitted by PHP Resque users over the last several years. A small handful of PRs remain unpulled (
chrisboulton/php-resque#35, chrisboulton/php-resque#81, chrisboulton/php-resque#338), but they're complex enough to deserve their own, separate PRs to this repo, so I've mentioned that to their creators (in the original PRs), and hopefully they'll bring them over here.