panicbit/fcm-rust

`Client::send()` should return a lazy future, not an outer code

Closed this issue · 4 comments

The example of fcm-rust usage from docs shows that the one is expected to use futures::future::lazy() to prepare a future which can then be spawned, however it's hardly possible in a real backend code as the Client is not Clone, i.e. each time when the user wants to spawn a notification, the whole hyper::Client will have to be moved inside the future for just one request.

It does not make sense to clone Client though as that would probably be too expensive, so if we want to return a "just a future" which makes the job done, it would make sense to wrap the future which the Client returns into Lazy inside the implementation.

The example is not the best, I agree. You don't need to move the client though, like is done in the example found in this repo.

Hyper master works with async/await already, the tls part is about to upgrade soonish. My idea is to upgrade this, the a2 and web-push libraries to use the new syntax so that would be the right time to update the examples too in the docs.

But no, you don't have to and you should not move the client for every request.

Also did you read this example codebase on how to use these libraries to do a consumer that sends notifications?

The example is not the best, I agree. You don't need to move the client though, like is done in the example found in this repo.

Yes, for sure. That's how I'm currently using your library.

But if you were to write a lazy future which can be spawned by some external executor (like in the similar case from the example I referred to), then it would have required a 'static future, so the client would have to be moved in this case.

My only concern was that calling Client::send() would do some job before returning the future. I.e. there are 2 common ways how the function returning future may work: some libraries do return a future which the one has to spawn to do some useful result, others start doing something and return a future which the one has to spawn in order to complete it, in other words, the latter approach does require the runtime to exist at the moment when function (send()) is called. Not a big deal for my purposes though ;)

So when reading my old code here, it seems the only work that's done is the creation of the builder and building a request. After that it's all futures and should be polled...

Ok, ok. I just thought that it's akin to snapview/tokio-tungstenite#55