Skip to content
This repository was archived by the owner on Jan 9, 2023. It is now read-only.

Conversation

stephen-palmer
Copy link
Contributor

Fix a couple of high severity bugs:

Copy link

@pullrequest pullrequest bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ A review job has been created and sent to the PullRequest network.


Check the status or cancel PullRequest code review here - or - cancel by adding [!pr] to the title of the pull request.

@coveralls
Copy link

Coverage Status

Coverage decreased (-1.1%) to 91.648% when pulling 3a5a111 on release/v6.2.4 into e320d5c on master.

Copy link

@pullrequest pullrequest bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if it's a great idea to restart something that was killed with SIGKILL, but I don't have all of the background information on the situation. I think from a code quality perspective, this looks good.


Was this helpful? Yes | No

@stephen-palmer
Copy link
Contributor Author

stephen-palmer commented Jan 10, 2019

As these are sub-processes and managed by the master node process, I think it's correct to allow the master to restart sub-processes killed externally via SIGKILL. Sending SIGKILL to the master will terminate the main process and all sub-processes immediately.

@stephen-palmer stephen-palmer merged commit 545e286 into master Jan 10, 2019
@stephen-palmer stephen-palmer deleted the release/v6.2.4 branch February 6, 2019 19:18
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants