BRO-141: Implement Async Chatango#8
Open
toddbirchard wants to merge 3 commits into
Open
Conversation
There was a problem hiding this comment.
Pull request overview
This PR migrates broiestbot toward an asyncio-based Chatango client, updates DB access patterns to use per-call SQLAlchemy sessions (instead of a global session), and introduces an ASGI/uvicorn runtime entrypoint.
Changes:
- Replaced legacy in-repo Chatango implementation with an async Chatango client API and updated moderation/bot handlers to
async/await. - Refactored DB persistence/query code to use
Session()/Session.begin()contexts (better isolation, safer concurrency) and added DB-focused integration tests. - Switched the local run target from gunicorn to uvicorn and added an ASGI lifespan app to run the bot inside uvicorn’s loop.
Reviewed changes
Copilot reviewed 21 out of 23 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
tests/test_database.py |
Adds DB integration tests for persistence + DB fetch helpers + threaded execution path. |
tests/conftest.py |
Adds shared fixtures and post-test cleanup for DB rows written by tests. |
tests/__init__.py |
Declares test package. |
pyproject.toml |
Adds chatango-lib dependency and replaces gunicorn with uvicorn. |
poetry.lock |
Locks new dependencies (chatango-lib, uvicorn) and removes gunicorn. |
Makefile |
Updates make run to start uvicorn with the ASGI app + lifespan. |
database/__init__.py |
Moves to scoped sessions via sessionmaker and improves engine pooling options. |
chatango/ch.py |
Removes the legacy bundled Chatango implementation. |
chatango/_ws.py |
Removes the legacy websocket/frame helper code. |
broiestbot/moderation/users.py |
Converts moderation actions to async Chatango client APIs and updates room/message types. |
broiestbot/moderation/phrases.py |
Converts phrase moderation (delete + warn) to async room APIs. |
broiestbot/moderation/ban.py |
Converts ban behavior to async room APIs. |
broiestbot/data/users.py |
Refactors user persistence to use Session.begin() and reorganizes logic for async bot flow. |
broiestbot/data/chats.py |
Refactors chat persistence to use Session.begin() and early-return guards. |
broiestbot/commands/weather.py |
Switches weather emoji lookup to a scoped session query. |
broiestbot/commands/polls/tovala.py |
Switches poll DB lookup to a scoped session query. |
broiestbot/commands/polls/change.py |
Updates Chatango room import for the new client library. |
broiestbot/commands/polls/bachelor.py |
Removes unused global-session import. |
broiestbot/commands/footy/util.py |
Switches preferred-timezone lookup to a scoped session query. |
broiestbot/bot.py |
Migrates main bot to async Chatango client, adds DB fetch helpers, and threads blocking work. |
broiestbot/__init__.py |
Removes previous start_bot/join logic (entrypoint behavior changes). |
asgi.py |
Adds ASGI app with lifespan to run the bot as a uvicorn-managed background task. |
Comments suppressed due to low confidence (3)
broiestbot/init.py:2
broiestbot/__init__.pyno longer exportsstart_bot, butwsgi.pyimportsstart_botto build the app. This will raiseImportErrorat startup; either restorestart_bot/join_roomshere or updatewsgi.py/the Poetry script entrypoint to use the new ASGI app.
"""Initialize bot."""
broiestbot/commands/weather.py:159
get_weather_emojidereferencesweather_emoji.group/weather_emoji.iconin branches that run whenweather_emojiisNone(i.e., when no DB row matches the code). This will raiseAttributeError; handle theNonecase before accessing attributes.
with Session() as db:
weather_emoji = db.query(Weather).filter(Weather.code == weather_code).one_or_none()
if weather_emoji is not None:
return weather_emoji.icon
elif is_day == "no" and weather_emoji.group in [
"sun",
None,
]:
return emojize(":night_with_stars:", language="en")
elif weather_emoji.icon and is_day == "no":
return weather_emoji.icon
return ":sun:"
broiestbot/moderation/users.py:90
ignored_userreturns a user-facing string containing profanity. If this is meant to be displayed to end users, consider using neutral wording (and, if needed, keep harsher language only in internal logs) to avoid policy/abuse issues.
return emojize(
f":wave: @{user_name} bot privileges REVOKED for acting like a CUNT :wave:",
language="en",
)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
22
to
25
| [tool.poetry.dependencies] | ||
| python = ">=3.12,<4.0" | ||
| chatango-lib = {path = "https://github.com/toddbirchard/chatango-lib", develop = true} | ||
| requests = "*" |
Comment on lines
+114
to
+116
| def __init__(self, username: str = "", password: str = "", rooms: list = []): | ||
| super().__init__(username=username, password=password, rooms=rooms) | ||
| self.bot_username = username |
Comment on lines
381
to
383
| if user_name in CHATANGO_IGNORED_USERS or message.ip in CHATANGO_IGNORED_IPS: | ||
| return ignored_user(user_name, message.ip) | ||
| if chat_message == "!!": |
Comment on lines
271
to
273
| elif cmd_type == "changeorstayvote": | ||
| return change_or_stay_vote(user_name, content, room) | ||
| return change_or_stay_vote(user_name, content, None) | ||
| elif cmd_type == "changeorstay": |
Comment on lines
+17
to
+35
| def persist_user_data(room_name: str, user: User, message: RoomMessage, bot_username: str) -> None: | ||
| """ | ||
| Persist user metadata. | ||
|
|
||
| :param str room_name: Chatango room. | ||
| :param User user: User responsible for triggering command. | ||
| :param Message message: User submitted message. | ||
| :param RoomMessage message: User submitted message. | ||
| :param str bot_username: Name of the currently run bot. | ||
|
|
||
| :returns: None | ||
| """ | ||
| if not message.ip or not PERSIST_USER_DATA or bot_username not in ("broiestbro", "broiestbot"): | ||
| return | ||
| # Fetch geo data before opening a DB session to avoid holding the connection | ||
| # open during a potentially slow HTTP call. | ||
| existing_user = _check_existing_user(room_name, user, message) | ||
| if existing_user is not None: | ||
| return | ||
| user_data = geo.lookup_user_by_ip(message.ip) |
Comment on lines
+8
to
+28
| from database import Session | ||
| from database.models import Chat, ChatangoUser | ||
|
|
||
| TEST_USERNAME_PREFIX = "__pytest__" | ||
|
|
||
|
|
||
| @pytest.fixture(scope="session") | ||
| def event_loop(): | ||
| loop = asyncio.new_event_loop() | ||
| yield loop | ||
| loop.close() | ||
|
|
||
|
|
||
| @pytest.fixture(autouse=True) | ||
| def cleanup_test_rows(): | ||
| """Remove any test rows written during a test.""" | ||
| yield | ||
| with Session() as db: | ||
| db.query(Chat).filter(Chat.username.like(f"{TEST_USERNAME_PREFIX}%")).delete() | ||
| db.query(ChatangoUser).filter(ChatangoUser.username.like(f"{TEST_USERNAME_PREFIX}%")).delete() | ||
| db.commit() |
Comment on lines
+99
to
+101
| def _check_existing_user(room_name: str, user: User, message: RoomMessage) -> Optional[ChatangoUser]: | ||
| """Return existing user record if one exists for this room and IP, else None.""" | ||
| with Session() as db: |
| from broiestbot.data.chats import persist_chat_logs | ||
| from broiestbot.data.users import persist_user_data | ||
| from database import Session | ||
| from database.models import Chat, ChatangoUser |
Comment on lines
128
to
137
| def create_message( | ||
| self, | ||
| cmd_type, | ||
| content, | ||
| command: Optional[str] = None, | ||
| args: Optional[str] = None, | ||
| room: Optional[Room] = None, | ||
| room_name: Optional[str] = None, | ||
| user_name: Optional[str] = None, | ||
| bot_username: Optional[str] = None, | ||
| ) -> Optional[str]: |
Comment on lines
441
to
456
| cmd, args = self._parse_command(chat_message[1::].strip()) | ||
| command = session.query(Command).filter(Command.command == cmd).first() | ||
| command = await asyncio.to_thread(_db_fetch_command, cmd) | ||
| if command is not None and command.type != "reserved": | ||
| response = self.create_message( | ||
| response = await asyncio.to_thread( | ||
| self.create_message, | ||
| command.type, | ||
| command.response, | ||
| command=cmd, | ||
| args=args, | ||
| room=room, | ||
| room_name=room.name.lower(), | ||
| user_name=user_name, | ||
| bot_username=self.username.lower(), | ||
| ) | ||
| if response: | ||
| room.message(response, html=True) | ||
| await room.send_message(response, use_html=True) | ||
| else: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
WIP