2.0 #59
2.0 #59
Conversation
- logging - configuration - hc - telegram echo
src/main.py
Outdated
There was a problem hiding this comment.
Does this module called "src"?
It seems more obvious to use project name and leave root directory for various tech stuff:
docs/
examples/
url-fairy-bot/
tests/
routes/
handler.py
health.py
Dockerfile
main.py
config.py
README.md
src/main.py
Outdated
| logger = logging.getLogger(__name__) | ||
|
|
||
| # Initialize FastAPI | ||
| REST_API = FastAPI() |
There was a problem hiding this comment.
UPPERCASE indicates constant in Python, which is quite confusing here 🤔
| REST_API = FastAPI() | |
| api = FastAPI() |
src/__init.__py
Outdated
There was a problem hiding this comment.
This file would probably never be included somewhere (that's not a library project), so __init__.py is not required
Also, there's a typo in file extension
src/domain_replacements.py
Outdated
| domain_replacements = { | ||
| "https://twitter.com/": "https://www.fxtwitter.com/", | ||
| "https://www.instagram.com/p/": "https://www.ddinstagram.com/p/", | ||
| "https://www.instagram.com/reel/": "https://www.ddinstagram.com/reel/", | ||
| "https://www.twitter.com/": "https://www.fxtwitter.com/", | ||
| "https://x.com/": "https://www.fxtwitter.com/", | ||
| } |
There was a problem hiding this comment.
Here we use number of aliases to later replace them with a single value
Another way to do so is composing a dict of mask-replacement pairs:
| domain_replacements = { | |
| "https://twitter.com/": "https://www.fxtwitter.com/", | |
| "https://www.instagram.com/p/": "https://www.ddinstagram.com/p/", | |
| "https://www.instagram.com/reel/": "https://www.ddinstagram.com/reel/", | |
| "https://www.twitter.com/": "https://www.fxtwitter.com/", | |
| "https://x.com/": "https://www.fxtwitter.com/", | |
| } | |
| url_replacements_dict = { | |
| r"https://(www\.)?(twitter|x)\.com/": "https://www.fxtwitter.com/", | |
| r"https://(www\.)?instagram\.com/(p|reel)/": "https://www.ddinstagram.com/\2/", | |
| } | |
| def replace_url(): | |
| for mask, replacement in url_replacements_dict.items(): | |
| if re.compile(mask).match(url): | |
| url = re.sub(mask, replacement, url) | |
| return url | |
| return url |
src/healthcheck.py
Outdated
There was a problem hiding this comment.
I'd propose to place these routes in routes/ dir
src/message_handler.py
Outdated
| healthcheck_result = perform_healthcheck() | ||
| await message.reply(healthcheck_result) | ||
| return | ||
| if (message.text.strip() == "/start") or (message.text.strip() == "/help"): |
There was a problem hiding this comment.
Same as above, process with explicit handler and filter for specific command
| license = "GPL v3" | ||
| readme = "README.md" | ||
|
|
||
| [tool.poetry.dependencies] |
There was a problem hiding this comment.
No explicit aiogram package installation?
src/message_handler.py
Outdated
| logger.debug("✨ Message received: %s", message.text) | ||
|
|
||
| error_messages = [] | ||
| if message.chat.type == types.ChatType.PRIVATE: |
There was a problem hiding this comment.
This may be also filtered:
https://docs.aiogram.dev/en/latest/dispatcher/filters/magic_filters.html#custom-function
src/message_handler.py
Outdated
|
|
||
| async def handle_url(url, message): | ||
| logger.debug("✨ Handling url: %s from %s", url, message) | ||
| time.sleep(5) |
There was a problem hiding this comment.
What's purpose of sleeping here?
There was a problem hiding this comment.
That's debugging line to find out does it parse links in one message at the same time or waiting before parsing the second one. (it waits)
There was a problem hiding this comment.
Yeah, you need to send tasks to another routine explicitly:
- network job - https://docs.python.org/3/library/asyncio-task.html#asyncio.create_task
- i/o job - https://docs.python.org/3/library/asyncio-task.html#asyncio.to_thread
(Also see another comment about threads and healthcheck)
src/main.py
Outdated
| fastapi_thread = Thread(target=start_fastapi) | ||
| fastapi_thread.start() | ||
|
|
||
| # Keep the main thread running |
There was a problem hiding this comment.
Not sure, why 2 threads?
As I see, we use aiogram in polling mode here
What's the point of using fastapi then?
There was a problem hiding this comment.
Can you show the example how to make a healthcheck and the main code work in parallel?
There was a problem hiding this comment.
If we produce heavy tasks, we can send those to another thread using asyncio:
https://docs.python.org/3/library/asyncio-task.html#asyncio.create_task
graph LR
telegram((telegram))
subgraph bot
poll_loop
handler
heavy_task
end
poll_loop -->|getUpdate| telegram
poll_loop -->|handleUpdate| handler
handler -.->|asyncio.create_task| heavy_task
This creates separate routine to perform the task not blocking the main loop.
If we need e.g. healthchecks and metrics to be accessible from outside, then it may be good idea to compose bot in webhook mode:
https://docs.aiogram.dev/en/latest/dispatcher/webhook.html#examples
graph LR
telegram((telegram))
orchestrator((orchestrator))
monitoring((monitoring))
subgraph bot
subgraph loop["main loop"]
handler["/handler"]
healthz["/healthz"]
metrics["/metrics"]
end
heavy_task
end
telegram -->|sendUpdate| handler
handler -.->|asyncio.create_task| heavy_task
orchestrator -.-> healthz
monitoring -.-> metrics
Adding FastAPI to process just /health requests seems kinda costly way to do the job
process more than one url
WiP