Start to support php-http #91
Conversation
In order to use the same feature through different implementation
tyx
force-pushed
the
feature/support-php-http
branch
from
b4a57f9
to
68a0441
Compare
composer.json
Outdated
| @@ -7,7 +7,8 @@ | |||
| "require": { | |||
| "ramsey/uuid": "^3.1", | |||
| "symfony/psr-http-message-bridge": "^1.0", | |||
| "zendframework/zend-diactoros": "^1.3" | |||
| "zendframework/zend-diactoros": "^1.3", | |||
| "php-http/mock-client": "^0.3.3" | |||
There was a problem hiding this comment.
Is there any specific reason this is not a dev requirement? It's for testing.
| @@ -105,6 +105,11 @@ private function onTerminate(PromiseOperation $operation, ThrowableCatcherVoter | |||
| return $fulfilled ? new FulfilledPromise($value) : new RejectedPromise($value); | |||
| } | |||
|
|
|||
| // If it is a php http Promise, use php http's objects. | |||
| if ($promise instanceof \Http\Promise\Promise) { | |||
| return $fulfilled ? new \Http\Client\Promise\HttpFulfilledPromise($value) : new \Http\Client\Promise\HttpRejectedPromise($value); | |||
There was a problem hiding this comment.
I think there is some confusion here: $promise here is the same as you call the on terminate callback on. Why? Since RetryPromiseOperationRunner::run returns a promise which receives a promise in it's callback, $value itself in this case will be a promise, likely unresolved.
Why do you need to specifically set Fulfilled/Rejected promises here? It's useful when you already know the resolution which actually you don't: $promise is fulfilled, but there is no guarantee that $value is too, so you will have to call ->wait() here as @joelwurtz pointed, but in that case you will loose real async.
But I would advise reviewing how promises work, seems to be a bit complicated to me. There are a few examples in PHP-HTTP documentation, Guzzle is a bit more sophisticated, but basically the same, you might want to check it's documentation out too.
tyx
force-pushed
the
feature/support-php-http
branch
from
68a0441
to
47dbfa6
Compare
tyx
force-pushed
the
feature/support-php-http
branch
from
47dbfa6
to
feba3e4
Compare
Thanks to @iamluc the implementation was trivial.
However, this one will not support real async as php-http does not implement it.
Missing points