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

Use LazyPromise? #163

Closed
prolic opened this issue Jun 30, 2017 · 6 comments
Closed

Use LazyPromise? #163

prolic opened this issue Jun 30, 2017 · 6 comments

Comments

@prolic
Copy link
Member

prolic commented Jun 30, 2017

public function __invoke(Message $message, Deferred $deferred = null):?Promise
{
    $needReply = (bool) $deferred;
    // stripped code. 
    if ($needReply) { 
        return new LazyPromise(function() use ($deferred, $reply) {
            $value = JSON::decode($reply->receive($this->replyTimeout)->getBody());

            try {
                $deferred->resolve($value);
            } catch (TimeoutException $e) {
                $deferred->reject($e->getMessage());
            }
        });
    }

But this is a BC an can only be done with a new major release.

@makasim
Copy link
Member

makasim commented Jun 30, 2017

for ref reactphp/promise#95

@basz
Copy link
Member

basz commented Jun 30, 2017

'''
(bool) $deferred;
'''
?

@codeliner
Copy link
Member

codeliner commented Jun 30, 2017

Added a comment in the linked reactphp issue. LazyPromise does not really help and the code above won't work I guess.

See the reactphp example of LazyPromise

The factory (callback passed to LazyPromise) needs to create its own deferred and return the promise of that deferred.

So you cannot resolve the deferred injected by the query bus. But only the injected deferred has a connection to the promise returned by the query bus.

@prolic
Copy link
Member Author

prolic commented Jul 1, 2017

Can this be closed then?

@codeliner
Copy link
Member

issue is still discussed. Own LazyPromise implementation could provide the wanted behavior. See linked reactphp issue

@makasim
Copy link
Member

makasim commented Jul 1, 2017

There is no possibility to set a custom promise to the deferred given as argument (the interface does not allow it). We could achieve what we want if prooph producer interface defines a promise as a return value. I guess the deferred could be removed in that case.

It is BC break though

@prolic prolic closed this as completed Oct 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants