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

Consider supporting thenables #43

Closed
joshdifabio opened this issue May 6, 2016 · 12 comments
Closed

Consider supporting thenables #43

joshdifabio opened this issue May 6, 2016 · 12 comments

Comments

@joshdifabio
Copy link
Contributor

In order to use Aerys in front of my existing async application I have to convert all of my Guzzle and React promises to Amp promises, which means I have to invest a load of time writing boilerplate code before I can properly test out Aerys, which means I probably won't give it a proper try because that time will have been wasted if we subsequently decide not to use Aerys. Are there any plans to support thenables in the Amp coroutine implementation in order to make it easier to work with non-Amp libraries?

@kelunik
Copy link
Member

kelunik commented May 6, 2016

I don't think supporting Thenables in the coroutines would be an issue. It's more that we'd need an event loop adapter.

@joshdifabio
Copy link
Contributor Author

The thing is, going through an application and re-factoring all the code which is coupled to a specific event loop is far less work than re-factoring all the code which is coupled to a specific promise implementation.

The project I'm working on at present is deployed to AWS which means virtually all our services are accessed over HTTP. We pretty much only reference the event loop when setting up the HTTP clients in our DI config.

The thing which is making my life hard is the lack of interoperability with thenables.

@kelunik
Copy link
Member

kelunik commented May 6, 2016

The project I'm working on at present is deployed to AWS which means virtually all our services are accessed over HTTP. We pretty much only reference the event loop when setting up the HTTP clients in our DI config.

Where are these thenables created and resolved? They have to be coupled to the event loop somewhere.

The thing is, going through an application and re-factoring all the code which is coupled to a specific event loop is far less work than re-factoring all the code which is coupled to a specific promise implementation.

Can't you just write a function that creates a promise from a React thenable?

@bwoebi
Copy link
Member

bwoebi commented May 7, 2016

@kelunik It's not that easy, React libs register their events on the react loop too.
What we need is a full-blown adapter library.

(But I'm all in favor of finally doing that.)

@kelunik
Copy link
Member

kelunik commented May 7, 2016

@bwoebi: That's what I said.

@joshdifabio
Copy link
Contributor Author

joshdifabio commented May 7, 2016

I agree that an event loop adapter would be great, but I believe there is still a lot of value in supporting thenables in the coroutine implementation even without a reactor/loop adapter. In fact, my experience leads me to believe that promise interoperability provides far more real world value than event loop interoperability despite being much easier to implement (it would take five minutes).

Where are these thenables created and resolved? They have to be coupled to the event loop somewhere.

Firstly, I assume we'd all agree that none of the popular promise implementations in PHP are inherently coupled to a specific event loop, nor should they be.

But I don't think that's what you're saying, and maybe I need to provide more detail on my current situation and why I'm asking for this feature.

I'm currently leading a team of five which is working full time developing an asynchronous SaaS application which is deployed to AWS and written in PHP. The backend application currently comprises around 50 PHP packages. Only one of those packages requires a specific event loop implementation, but almost all of them work with thenables of different kinds.

Our application is stateless and consumes a number of services, most of which are AWS services. These include SQS (message queue), S3 (object store), DynamoDB (document store), SES (email service), Mandrill (another email service), keen.io (analytics). Guess how many places we reference the event loop at present? One place. That's because all of these services are interacted with over HTTP, which means all of our IO is over HTTP. We create a single Guzzle handler which is injected to Guzzle clients all over our application by our DI container. We define interfaces for things like document stores and distributed lock services, and then provide implementations of these interfaces using AWS services, using the AWS SDK, and finally wrap these services in higher-level services such as entity repositories. But none of these layers have a need to reference the event loop directly. The only place we touch the event loop is to periodically tick Guzzle's curl handler. To switch over to Amp's loop, I literally only have to change a few lines of code in a configuration package. As a future task I can potentially create a Guzzle handler using Artax (I don't see PSR 7 streams as a major barrier) if this provides greater efficiency.

As you can see, real-world code is massively more coupled to specific promise interfaces than it is to specific reactors, and so reactor/event loop interoperability, which I don't believe we'll have any time soon, doesn't really buy me much, despite being quite a lot of work.

Can't you just write a function that creates a promise from a React thenable?

My controllers return generators, so I would have to write a function which translates these generators from yielding thenables to yielding only Amp promises, which is far more complicated than adding thenable support to Amp's coroutine implementation, as well as creating an unnecessary performance hit.

@kelunik
Copy link
Member

kelunik commented May 7, 2016

There's now a possible PR: #44. But I'm not really happy with it. Currently it adds a dependency to react/promise. Unfortunately, they didn't separate the PromiseInterface from the other things, so we probably have to depend on ^1|^2|^3.

I still think it's better when you transform the react promises yourself before yielding them in an Amp coroutines. This ensures we keep our code base clean and don't have any issues going forward whatever ReactPHP decides to do.

@DaveRandom
Copy link
Contributor

https://github.com/DaveRandom/Loopio

Quite out of date but could probably be made to do the job rather than having to start from scratch.

@joshdifabio
Copy link
Contributor Author

https://github.com/DaveRandom/Loopio

Quite out of date but could probably be made to do the job rather than having to start from scratch.

Thanks for the heads up.

@bwoebi
Copy link
Member

bwoebi commented May 26, 2016

This will be part of Amp v2.0 which will implement the event-loop and awaitable standards.

@staabm
Copy link
Member

staabm commented May 27, 2016

just for reference. "standards" means what https://github.com/async-interop will define.

@kelunik
Copy link
Member

kelunik commented Jul 19, 2016

Closing as we won't support thenables, but the Awaitable standard then.

@kelunik kelunik closed this as completed Jul 19, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

5 participants