-
Notifications
You must be signed in to change notification settings - Fork 5
🔥 Feature: Add broadcast to bot and space #86
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
base: main
Are you sure you want to change the base?
Changes from all commits
683f14b
6645f52
8187689
8f7f81f
496e3f7
c2bf1c4
5491565
5688f08
f872dc0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,6 +7,9 @@ | |
|
|
||
| from nio import AsyncClient, Event, MatrixRoom | ||
|
|
||
| from matrix.message import Message | ||
| from matrix.types import File | ||
|
|
||
| from .room import Room, make_room | ||
| from .space import Space | ||
| from .group import Group | ||
|
|
@@ -423,3 +426,44 @@ async def _build_context(self, matrix_room: Room, event: Event) -> Context: | |
| ctx.command = cmd | ||
|
|
||
| return ctx | ||
|
|
||
| # ROOMS | ||
|
|
||
| async def broadcast( | ||
| self, | ||
| rooms: list[Room], | ||
| content: str | None = None, | ||
| *, | ||
| raw: bool = False, | ||
| notice: bool = False, | ||
| file: File | None = None, | ||
| ) -> list[Message]: | ||
| """Broadcasts a message to the specified rooms. | ||
|
|
||
| Supports text messages (with optional markdown formatting) | ||
| and file uploads (including images, videos, and audio). | ||
| If a space is provided, it is silently skipped. | ||
|
|
||
| ## Example | ||
|
|
||
| ```python | ||
| # Broadcast a markdown-formatted text message | ||
| await bot.broadcast([room1, room2, ...], "Hello **world**!") | ||
|
|
||
| # Broadcast a notice message | ||
| await bot.broadcast([room1, room2, ...], "Event started", notice=True) | ||
|
|
||
| # Broadcast a file | ||
| file = File(path="mxc://...", filename="document.pdf", mimetype="application/pdf") | ||
| await bot.broadcast([room1, room2, ...], file=file) | ||
|
|
||
| # Broadcast an image | ||
| image = Image(path="mxc://...", filename="photo.jpg", mimetype="image/jpeg", width=800, height=600) | ||
| await bot.broadcast([room1, room2, ...], file=image) | ||
| ``` | ||
| """ | ||
| rooms = list(filter(lambda child: not isinstance(child, Space), rooms)) | ||
| async_send = [ | ||
| room.send(content, raw=raw, notice=notice, file=file) for room in rooms | ||
| ] | ||
| return await asyncio.gather(*async_send) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's a good chance we get rate limited doing this, so we need to think about how to avoid it. Here's what I have in mind. To avoid rate limiting, we first need to implement this (the BroadcastResult return type), so a rate-limited room doesn't blow up the whole broadcast and we have a place to report per-room failures. Then, broadcast would look something like: async def broadcast(self, rooms, content=None, *, max_concurrent=8, **kwargs) -> list[BroadcastResult]:
semaphore = asyncio.Semaphore(max_concurrent)
rooms = filter(lambda r: not isinstance(r, Space), rooms)
async def _send(room):
async with semaphore:
try:
msg = await with_retry(lambda: room.send(content, **kwargs))
return BroadcastResult(room=room, message=msg, error=None)
except Exception as e:
return BroadcastResult(room=room, message=None, error=e)
return await asyncio.gather(*[_send(room) for room in rooms])It's also a good opportunity to add a new helper in async def with_retry(coro, *, retries=3):
...I am also debating if it would be worth it to extract the semaphore logic into another async def bounded_gather(coros, *, max_concurrent=8):
semaphore = asyncio.Semaphore(max_concurrent)
async def _run(coro):
async with semaphore:
return await coro
return await asyncio.gather(*[_run(c) for c in coros])This is mainly for consistency with the with_retry and to avoid too much repetitions but it's not blocking either if it's directly in the broadcast so maybe we should Of course, all this would require some tests as well. As a FYI, the
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using So, I think in the case of broadcasting, it would be more relevant to set @dataclass
class BroadcastResult:
room: Room
message: Message | None
error: Exception | None
@property
def ok(self) -> bool:
return self.error is None
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,11 @@ | ||
| import asyncio | ||
|
|
||
| from matrix.message import Message | ||
| from typing import Self | ||
| from matrix.room import Room, make_room | ||
|
|
||
| from matrix.types import File | ||
|
|
||
|
|
||
| class Space(Room, room_type="m.space"): | ||
| def get_children(self, depth: int = 1) -> list[Room | Self]: | ||
|
|
@@ -43,3 +48,50 @@ def get_children(self, depth: int = 1) -> list[Room | Self]: | |
| children.extend(child.get_children(depth - 1)) | ||
|
|
||
| return children | ||
|
|
||
| async def broadcast( | ||
| self, | ||
| content: str | None = None, | ||
| *, | ||
| raw: bool = False, | ||
| notice: bool = False, | ||
| file: File | None = None, | ||
| depth: int = 1, | ||
| ) -> list[Message]: | ||
| """Broadcasts a message to all rooms in this space. | ||
|
|
||
| Supports text messages (with optional markdown formatting) | ||
| and file uploads (including images, videos, and audio). | ||
|
|
||
| Children the bot has not joined are silently omitted. Use `depth` to | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The doc says that space are silently omitted and the code seems to do just that but there is not test for it.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am not sure if this is possible; we should validate that children of space can return children that the bot isn't in. I also don't see where it does that in the code. If we don't want that we can remove that part of the comment. If we do, we need a test to validate that and something that actually check it. But as in the earlier comment, with the changes I propose we might be able to let if fail without too much cost. We probably still want a test because it might be a recurring edge case. |
||
| recursively broadcast to children of sub-spaces. `depth=1` broadcasts | ||
| to direct children only (default). | ||
|
|
||
| ## Example | ||
|
|
||
| ```python | ||
| # Broadcast a markdown-formatted text message | ||
| await space.broadcast("Hello **world**!") | ||
|
|
||
| # Broadcast a notice message | ||
| await space.broadcast("Event started", notice=True) | ||
|
|
||
| # Broadcast a file | ||
| file = File(path="mxc://...", filename="document.pdf", mimetype="application/pdf") | ||
| await space.broadcast(file=file) | ||
|
|
||
| # Broadcast an image | ||
| image = Image(path="mxc://...", filename="photo.jpg", mimetype="image/jpeg", width=800, height=600) | ||
| await space.broadcast(file=image) | ||
|
|
||
| # Broadcast a notice message to space's rooms and the rooms of its subspaces | ||
| await space.broadcast("New Announcement", notice=True, depth=2) | ||
| ``` | ||
| """ | ||
| rooms = filter( | ||
| lambda room: not isinstance(room, Space), self.get_children(depth=depth) | ||
| ) | ||
| async_send = [ | ||
| room.send(content, raw=raw, notice=notice, file=file) for room in rooms | ||
| ] | ||
| return await asyncio.gather(*async_send) | ||
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.
It's weird that the space version checks if the bot isn't in the room (and skip them). It's even more important to validate this here.
Although, with the changes I propose we might be able to let it fail but it's probably better to avoid it still.