refactor: improve code readability and structure in various components
- Refactored the `mini_app_short_name` assignment in `config.py` for better clarity. - Enhanced the `app_config_js` function in `app.py` to improve formatting of the JavaScript response body. - Added per-chat locks in `group_duty_pin.py` to prevent concurrent refreshes, improving message handling. - Updated `_schedule_next_update` to include optional jitter for scheduling, enhancing performance during high-load scenarios. - Cleaned up test files by removing unused imports and improving test descriptions for clarity.
This commit is contained in:
@@ -2,7 +2,8 @@
|
||||
|
||||
import asyncio
|
||||
import logging
|
||||
from datetime import datetime, timezone
|
||||
import random
|
||||
from datetime import datetime, timedelta, timezone
|
||||
from typing import Literal
|
||||
|
||||
import duty_teller.config as config
|
||||
@@ -29,6 +30,10 @@ from duty_teller.services.group_duty_pin_service import (
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
# Per-chat locks to prevent concurrent refresh for the same chat (avoids duplicate messages).
|
||||
_refresh_locks: dict[int, asyncio.Lock] = {}
|
||||
_lock_for_refresh_locks = asyncio.Lock()
|
||||
|
||||
JOB_NAME_PREFIX = "duty_pin_"
|
||||
RETRY_WHEN_NO_DUTY_MINUTES = 15
|
||||
|
||||
@@ -124,8 +129,12 @@ def _sync_untrust_group(chat_id: int) -> tuple[bool, int | None]:
|
||||
|
||||
|
||||
async def _schedule_next_update(
|
||||
application, chat_id: int, when_utc: datetime | None
|
||||
application,
|
||||
chat_id: int,
|
||||
when_utc: datetime | None,
|
||||
jitter_seconds: float | None = None,
|
||||
) -> None:
|
||||
"""Schedule the next pin refresh job. Optional jitter spreads jobs when scheduling many chats."""
|
||||
job_queue = application.job_queue
|
||||
if job_queue is None:
|
||||
logger.warning("Job queue not available, cannot schedule pin update")
|
||||
@@ -136,8 +145,10 @@ async def _schedule_next_update(
|
||||
if when_utc is not None:
|
||||
now_utc = datetime.now(timezone.utc).replace(tzinfo=None)
|
||||
delay = when_utc - now_utc
|
||||
if jitter_seconds is not None and jitter_seconds > 0:
|
||||
delay += timedelta(seconds=random.uniform(0, jitter_seconds))
|
||||
if delay.total_seconds() < 1:
|
||||
delay = 1
|
||||
delay = timedelta(seconds=1)
|
||||
job_queue.run_once(
|
||||
update_group_pin,
|
||||
when=delay,
|
||||
@@ -146,8 +157,6 @@ async def _schedule_next_update(
|
||||
)
|
||||
logger.info("Scheduled pin update for chat_id=%s at %s", chat_id, when_utc)
|
||||
else:
|
||||
from datetime import timedelta
|
||||
|
||||
job_queue.run_once(
|
||||
update_group_pin,
|
||||
when=timedelta(minutes=RETRY_WHEN_NO_DUTY_MINUTES),
|
||||
@@ -168,11 +177,13 @@ async def _refresh_pin_for_chat(
|
||||
|
||||
Uses single DB session for message_id, text, next_shift_end (consolidated).
|
||||
If the group is no longer trusted, removes pin record, job, and message; returns "untrusted".
|
||||
Unpin is best-effort (e.g. if user already unpinned we still pin the new message and save state).
|
||||
Per-chat lock prevents concurrent refresh for the same chat.
|
||||
|
||||
Returns:
|
||||
"updated" if the message was sent, pinned and saved successfully;
|
||||
"no_message" if there is no pin record for this chat;
|
||||
"failed" if send_message or permissions failed;
|
||||
"failed" if send_message or pin failed;
|
||||
"untrusted" if the group was removed from trusted list (pin record and message cleaned up).
|
||||
"""
|
||||
loop = asyncio.get_running_loop()
|
||||
@@ -205,42 +216,59 @@ async def _refresh_pin_for_chat(
|
||||
logger.info("No pin record for chat_id=%s, skipping update", chat_id)
|
||||
return "no_message"
|
||||
old_message_id = message_id
|
||||
|
||||
async with _lock_for_refresh_locks:
|
||||
lock = _refresh_locks.setdefault(chat_id, asyncio.Lock())
|
||||
try:
|
||||
msg = await context.bot.send_message(
|
||||
chat_id=chat_id,
|
||||
text=text,
|
||||
reply_markup=_get_contact_button_markup(config.DEFAULT_LANGUAGE),
|
||||
)
|
||||
except (BadRequest, Forbidden) as e:
|
||||
logger.warning(
|
||||
"Failed to send duty message for pin refresh chat_id=%s: %s", chat_id, e
|
||||
)
|
||||
await _schedule_next_update(context.application, chat_id, next_end)
|
||||
return "failed"
|
||||
try:
|
||||
await context.bot.unpin_chat_message(chat_id=chat_id)
|
||||
await context.bot.pin_chat_message(
|
||||
chat_id=chat_id,
|
||||
message_id=msg.message_id,
|
||||
disable_notification=not config.DUTY_PIN_NOTIFY,
|
||||
)
|
||||
except (BadRequest, Forbidden) as e:
|
||||
logger.warning("Unpin or pin after refresh failed chat_id=%s: %s", chat_id, e)
|
||||
await _schedule_next_update(context.application, chat_id, next_end)
|
||||
return "failed"
|
||||
await loop.run_in_executor(None, _sync_save_pin, chat_id, msg.message_id)
|
||||
if old_message_id is not None:
|
||||
try:
|
||||
await context.bot.delete_message(chat_id=chat_id, message_id=old_message_id)
|
||||
except (BadRequest, Forbidden) as e:
|
||||
logger.warning(
|
||||
"Could not delete old pinned message %s in chat_id=%s: %s",
|
||||
old_message_id,
|
||||
chat_id,
|
||||
e,
|
||||
)
|
||||
await _schedule_next_update(context.application, chat_id, next_end)
|
||||
return "updated"
|
||||
async with lock:
|
||||
try:
|
||||
msg = await context.bot.send_message(
|
||||
chat_id=chat_id,
|
||||
text=text,
|
||||
reply_markup=_get_contact_button_markup(config.DEFAULT_LANGUAGE),
|
||||
)
|
||||
except (BadRequest, Forbidden) as e:
|
||||
logger.warning(
|
||||
"Failed to send duty message for pin refresh chat_id=%s: %s",
|
||||
chat_id,
|
||||
e,
|
||||
)
|
||||
await _schedule_next_update(context.application, chat_id, next_end)
|
||||
return "failed"
|
||||
try:
|
||||
await context.bot.unpin_chat_message(chat_id=chat_id)
|
||||
except (BadRequest, Forbidden) as e:
|
||||
logger.debug(
|
||||
"Unpin failed (e.g. no pinned message) chat_id=%s: %s", chat_id, e
|
||||
)
|
||||
try:
|
||||
await context.bot.pin_chat_message(
|
||||
chat_id=chat_id,
|
||||
message_id=msg.message_id,
|
||||
disable_notification=not config.DUTY_PIN_NOTIFY,
|
||||
)
|
||||
except (BadRequest, Forbidden) as e:
|
||||
logger.warning("Pin after refresh failed chat_id=%s: %s", chat_id, e)
|
||||
await _schedule_next_update(context.application, chat_id, next_end)
|
||||
return "failed"
|
||||
await loop.run_in_executor(None, _sync_save_pin, chat_id, msg.message_id)
|
||||
if old_message_id is not None:
|
||||
try:
|
||||
await context.bot.delete_message(
|
||||
chat_id=chat_id, message_id=old_message_id
|
||||
)
|
||||
except (BadRequest, Forbidden) as e:
|
||||
logger.warning(
|
||||
"Could not delete old pinned message %s in chat_id=%s: %s",
|
||||
old_message_id,
|
||||
chat_id,
|
||||
e,
|
||||
)
|
||||
await _schedule_next_update(context.application, chat_id, next_end)
|
||||
return "updated"
|
||||
finally:
|
||||
async with _lock_for_refresh_locks:
|
||||
_refresh_locks.pop(chat_id, None)
|
||||
|
||||
|
||||
async def update_group_pin(context: ContextTypes.DEFAULT_TYPE) -> None:
|
||||
@@ -338,12 +366,15 @@ def _get_all_pin_chat_ids_sync() -> list[int]:
|
||||
|
||||
|
||||
async def restore_group_pin_jobs(application) -> None:
|
||||
"""Restore scheduled pin-update jobs for all chats that have a pinned message (on startup)."""
|
||||
"""Restore scheduled pin-update jobs for all chats that have a pinned message (on startup).
|
||||
|
||||
Uses jitter (0–60 s) per chat to avoid thundering herd when many groups share the same shift end.
|
||||
"""
|
||||
loop = asyncio.get_running_loop()
|
||||
chat_ids = await loop.run_in_executor(None, _get_all_pin_chat_ids_sync)
|
||||
next_end = await loop.run_in_executor(None, _get_next_shift_end_sync)
|
||||
for chat_id in chat_ids:
|
||||
await _schedule_next_update(application, chat_id, next_end)
|
||||
await _schedule_next_update(application, chat_id, next_end, jitter_seconds=60.0)
|
||||
logger.info("Restored %s group pin jobs", len(chat_ids))
|
||||
|
||||
|
||||
@@ -407,6 +438,8 @@ async def pin_duty_cmd(update: Update, context: ContextTypes.DEFAULT_TYPE) -> No
|
||||
message_id=message_id,
|
||||
disable_notification=True,
|
||||
)
|
||||
next_end = await loop.run_in_executor(None, _get_next_shift_end_sync)
|
||||
await _schedule_next_update(context.application, chat_id, next_end)
|
||||
await update.message.reply_text(t(lang, "pin_duty.pinned"))
|
||||
except (BadRequest, Forbidden) as e:
|
||||
logger.warning("pin_duty failed chat_id=%s: %s", chat_id, e)
|
||||
|
||||
Reference in New Issue
Block a user