-
-
Notifications
You must be signed in to change notification settings - Fork 127
Added Redis Stream backend. #3
Conversation
| await asyncio.sleep(1) | ||
|
|
||
| async def next_published(self) -> Event: | ||
| await self._wait_for_streams() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here I want to wait for at least one stream.
Don't know is it a good approach or not?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't it work fine without the sleep? It should only stop reading if timeout is specified. I'll check it tomorrow
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, it's a bit complicated. For instance, we also want to properly cover the case where a new stream is subscribed too, while we're waiting on an existing xread.
14283e5 to
3cfcc8b
Compare
lovelydinosaur
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fantastic stuff! Therre's a bit of complexity around how we'll deal with waiting for/issuing/canceling the xread that'll need a bit of thinking over.
I'm wondering if a good precursor to this PR would be to first just implement an aioredis based PUB/SUB implementation. That looks to be more maintained that our existing dependency of asyncio-redis, and since it supports the stream API that asyncio-redis doesn't, it'd be a better default for us.
|
Sorry I have closed the PR by mistake. |
Yup. (I actually meant adding an |
|
Im very interested in this backend. Is there a way to push this forward without forking? |
|
@sarendsen Using your own fork and reporting any issues is a good way forward. |
No description provided.