Skip to content

Conversation

bosunski
Copy link

Summary of changes

  • Adds $hijak parameter to exectStartsStream
  • Configures the browser based on $hijak

Example Usage

$client->execCreate('bash', 'bash', true, true, true, true)
            ->then(function($info) use ($client) {
                $client->execStartStream($info['Id'], true,'stderr', true)->then(function (UpgradedResponse $e) use ($deferred) {
                    $stream = $e->getConnection();
                    $stream->on('data', function ($chunk) {
                        echo $chunk;
                    });

                    $stream->write("ls\r"); // list directories
                });
            });

Copy link
Owner

@clue clue left a comment

Choose a reason for hiding this comment

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

@bosunski Thanks for looking into this, I think this is an excellent first step to discuss this feature in mroe detail!

I love the feature, but I'm not sure about the API in its current form.

The execStartStream() currently returns a ReadableStreamInterface (read-only).

It's my understanding the new API that supports writable streams should return a DuplexStreamInterface instead.

The DuplexStreamInterface extends the ReadableStreamInterface, so we could make this the return value of the same function or it might be better to come up with a dedicated API endpoint, I'm currently undecided.

What do you think about this? 👍

@bosunski
Copy link
Author

bosunski commented Aug 20, 2020

@clue I'm thinking 🤔 since the Hijak feature is still part of the /exec/{exec}/start API and only surfaces when the Hijak is configured, we can just have the execStartStream return a DuplexStreamInterface which, as you've said, will cover for the ReadableStreamInterface also.

I initially added execStartStreamUpgrade as a public interface, but I noticed the same parameters are being passed for it as well as execStartStream which is more like some repetition there. Which lead me to merge the 2 methods.

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.

Support writable streams (process STDIN)

2 participants