Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @sentax - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 4 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.
| TELEGRAM_CHANNEL_ID = os.environ.get("TELEGRAM_CHANNEL_ID") | ||
| TELEGRAM_API_TOKEN = os.environ.get("TELEGRAM_API_TOKEN") |
There was a problem hiding this comment.
suggestion (bug_risk): Environment variables should have default values.
Consider providing default values for TELEGRAM_CHANNEL_ID and TELEGRAM_API_TOKEN to avoid potential NoneType errors if the environment variables are not set.
| TELEGRAM_CHANNEL_ID = os.environ.get("TELEGRAM_CHANNEL_ID") | |
| TELEGRAM_API_TOKEN = os.environ.get("TELEGRAM_API_TOKEN") | |
| TELEGRAM_CHANNEL_ID = os.environ.get("TELEGRAM_CHANNEL_ID", "") | |
| TELEGRAM_API_TOKEN = os.environ.get("TELEGRAM_API_TOKEN", "") |
| } | ||
| try: | ||
| response = requests.post(url, data=payload) | ||
| return {"ok":True} |
There was a problem hiding this comment.
suggestion: Consider logging the exception.
In the except block, consider logging the exception to help with debugging in case of failures.
| return {"ok":True} | |
| return {"ok": True} | |
| except Exception as e: | |
| logging.error(f"Exception occurred: {e}") | |
| return {"ok": False} |
core/telegram.py
Outdated
| log_hash = hash(message) | ||
| current_time = time.time() | ||
|
|
||
| # Check if the log has been sent before or if it has been more than 1 hour |
There was a problem hiding this comment.
issue (typo): Comment does not match the code.
The comment mentions '1 hour', but the code uses MIN_INTERVAL which is set to 10 seconds. Update the comment to accurately reflect the code logic.
| current_time = time.time() | ||
|
|
||
| # Check if the log has been sent before or if it has been more than 1 hour | ||
| if current_time - log_cache[log_hash] > MIN_INTERVAL: |
There was a problem hiding this comment.
issue (bug_risk): Potential race condition with log_cache.
Accessing and modifying log_cache without any locking mechanism could lead to race conditions in a multi-threaded environment. Consider using a thread-safe data structure or adding appropriate locking.
| res = self.middleware.log_message("Test log message") | ||
| self.assertEqual(res['ok'], True) | ||
| res = self.middleware.log_message("Test log message") | ||
| self.assertEqual(res['ok'], False) | ||
| # delay | ||
| time.sleep(TELEGRAM_MIN_LOG_INTERVAL) | ||
| res = self.middleware.log_message("Test log message") | ||
| self.assertEqual(res['ok'], True) | ||
| res = self.middleware.log_message("Test log message") | ||
| self.assertEqual(res['ok'], False) |
There was a problem hiding this comment.
issue (code-quality): Extract duplicate code into method (extract-duplicate-method)
sentax
force-pushed
the
feature/telegram-bot
branch
from
ad3bdb7 to
7f69504
Compare
1 participant
Implemented a feature for monitoring porpuses that uses telegram bot API and django middleware.
2 new env variables added:
TELEGRAM_API_TOKEN : father bot given token
TELEGRAM_CHANNEL_ID : channel id in 2 formats (number[-1001609474777], string["@ChannelID"])
bot should be added to the channel and allowed to send messages in that channel before.