Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Callback for exitCode #12

Merged
merged 8 commits into from Feb 25, 2019
Merged

Callback for exitCode #12

merged 8 commits into from Feb 25, 2019

Conversation

sarelvdwalt
Copy link
Contributor

Addresses #11 to create a callback in userland for exitCodes returned by processes.

@sarelvdwalt
Copy link
Contributor Author

The method chosen to do this was to add a configuration function setCompletedCallback. The way of using this is then:

$swarmProcess = new SwarmProcess($logger);
$swarmProcess->setCompletedCallback(function(Symfony\Component\Process\Process $process) {
    // do something with the $process returned, checking it's exit code, and perhaps putting it back on the stack
});

@sarelvdwalt sarelvdwalt marked this pull request as ready for review February 24, 2019 14:57
@sarelvdwalt
Copy link
Contributor Author

@mostertb and @johnathanmdell would you mind having a look and telling me what your thoughts are?

@johnathanmdell
Copy link

@sarelvdwalt I'm happy with that, nothing overly complicated to achieve some simple communication between the child and parent.

👍

@mostertb mostertb self-requested a review February 24, 2019 17:14
@mostertb
Copy link
Contributor

@sarelvdwalt This looks great!
Its something that I've personally very much wanted. Thank you

Two things:

  1. Will you update the README.md with this new functionality?
  2. If running in a loop with $swarm->tick(), the callback could theoretically be changed mid-execution. Is this something we intend to support?

@sarelvdwalt
Copy link
Contributor Author

@sarelvdwalt This looks great!
Its something that I've personally very much wanted. Thank you

Two things:

  1. Will you update the README.md with this new functionality?
  2. If running in a loop with $swarm->tick(), the callback could theoretically be changed mid-execution. Is this something we intend to support?

Thanks @mostertb

  1. Ok will do and notify
  2. Not sure, didn't think about this - I see no reason why not to allow this, other than protecting the user from themselves. I think we should leave this open.

@sarelvdwalt
Copy link
Contributor Author

@mostertb I have updated the documentation.

@sarelvdwalt
Copy link
Contributor Author

@mostertb and @johnathanmdell I have now moved the callback to the Configuration object. This object creation is also to plan for the future change planned for a setting to respect Process timeout settings.

@mostertb mostertb merged commit 1d92418 into afrihost:master Feb 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants