spacejam / sled

the champagne of beta embedded databases
Apache License 2.0
8.12k stars 383 forks source link

impl futures::Stream<Event> for Subscriber #1368

Open vi opened 3 years ago

vi commented 3 years ago

For some reason, sled::Subscriber implements Iterator and Future - a strange combination.

Instead of Future, futures::Stream would seem to be a more reasonable choice. How do I keep watching on more events asynchronously otherwise?

Use Case:

Watching multiple operations happening to a Sled database (like with Iterator impl), but asynchronously.

Proposed Change:

impl futures::Stream for Subscriber {
    type Item = Event;
    ....
}

Who Benefits From The Change(s)?

Async Sled users.

Alternative Approaches

asonix commented 3 years ago

As noted in the documentation, you can use

while let Some(item) = (&mut subscriber).await {
}

But you're right that a .next() method or similar would be more intuitive

vi commented 3 years ago

Typically resolved Futures should not be polled again:

Once a future has finished, clients should not poll it again.

Relying on reusing Future after non-Pending poll is like relying on Iterator becoming live again after already returning None once. Even if it works, it does not play nice with various combinators and hinders understanding the code.

futures::Stream is a canonical "multi-use Future".

busyboredom commented 2 years ago

This feature would also be very nice for Actix users. I had a hell of a time trying to use a Subscriber as a second stream for an actix websocket (I ended up just giving in and polling instead).

vi commented 2 years ago

@busyboredom , I expect that one can workaround it with a manual Stream implementation where poll_next delegates to Sled subscriber's poll.

tv42 commented 2 years ago

This sounds like a duplicate of https://github.com/spacejam/sled/issues/1311

The reworking of the idea to a sink sounds nice to me.