Skip to content

[Process] Provide interactive input. #19558

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

Closed

Conversation

boombatower
Copy link
Contributor

Q A
Branch? master
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
License MIT

Instead of always closing the input pipe after exhausting the input buffer or
resource leave the input pipe open.

  • setInputInteractive(): controls interactivity
  • appendInputBuffer(): convenience method for writing additional input
  • setInput(): now allows setting new input if interactive

Instead of always closing the input pipe after exhausting the input buffer or
resource leave the input pipe open.

- setInputInteractive(): controls interactivity
- appendInputBuffer(): convenience method for writing additional input
- setInput(): now allows setting new input if interactive
@boombatower
Copy link
Contributor Author

The diff is a tad misleading give that it looks like close() is being added when it in fact is untouched.

@nicolas-grekas
Copy link
Member

You can already do this by using an iterator as input, see
http://symfony.com/doc/current/components/process.html#streaming-to-the-standard-input-of-a-process

@boombatower
Copy link
Contributor Author

@nicolas-grekas Looks promising. I actually developed this on 3.0.x and rebased. Finally got around to cleaning up and trying to upstream. I'll take a look and close if nothing further.

@boombatower
Copy link
Contributor Author

It seems to work in principal, but interestingly it does not write every time ->isRunning() is called as my approach does.

My use-case is basically (paraphrased):

$process = new Process('cat');
$input = new InputStream();
$process->setInput($input);

$process->start(function ($type, $buffer) use($input) {
  echo "+ $buffer";

  if (/*some buffer output */) {
    $input->write(PHP_EOL); // continue
  }
}

while (true) {
    echo "main loop\n";
    $process->isRunning(); // triggers reading of pipes
    $process->checkTimeout(); // can trigger exception

    sleep(1);
}

main loop is output multiple times before the process responds with output buffer. With my implementation there is output between each loop iteration.

What I end up doing is manually writing to pipe via getPipes() (#19559) in order to have the response be written immediately. Instead of waiting for after sleep.

So to facilitate my usecase, using an InputStream and never writing to it should work fine to keep the input pipe open, but the current implementation seems to be a bit cumbersome to use given it is unclear when it chooses to write (it in fact differs how many loop iteration in output).

@nicolas-grekas
Copy link
Member

What about foreach ($process ....)?

@boombatower
Copy link
Contributor Author

boombatower commented Aug 7, 2016

That appears to flush the write buffer, but never times out since timeout is never checked (this feels like a bug). Additionally, for my usecase I need to do other things besides only wait on output from process (ie the stream_select() case describe in other issue).

@boombatower
Copy link
Contributor Author

To sum it up, the iterator input would work if there was a way to call readPipes() / $this->processPipes->readAndWrite() after calling write() on the InputStream.

@nicolas-grekas
Copy link
Member

Not sure stream_select fits into the component's scope. You may need to use proc_open directly, or use an abstraction provided by an async loop (e.g. ReactPHP)

@boombatower
Copy link
Contributor Author

proc_open() works, but then I found myself re-implementing the timeouts and many other features of process. Given all I really need is exactly what process is already doing except merging multiple process pipes together and a custom timeout.

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.

3 participants