Dev #127
Dev #127
Conversation
Reviewer's Guide by SourceryThis pull request refactors and optimizes several modules in the bot codebase, introduces new helper functions, and adds a GitHub Actions workflow for code formatting using Ruff. The changes include renaming functions, restructuring code for better readability, removing unused imports, and handling exceptions more gracefully. File-Level Changes
Tips
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @5hojib - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 9 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.
| await message.delete() | ||
|
|
||
| async def handle_done_action(id_, download, message): | ||
| await query.answer() |
There was a problem hiding this comment.
question (bug_risk): Potential missing argument in query.answer()
The query.answer() method can take arguments like text and show_alert. Ensure that this call is intended to be without arguments.
| else: | ||
| await handle_aria2_done(id_, download) | ||
|
|
||
| await sendStatusMessage(message) |
There was a problem hiding this comment.
question: Redundant message deletion
Since sendStatusMessage is followed by message.delete(), ensure that sendStatusMessage does not already delete the message to avoid redundant operations.
bot/modules/torrent_select.py
Outdated
| if file['selected'] == 'false' and await aiopath.exists(file['path']): | ||
| try: | ||
| await aioremove(file['path']) | ||
| except: |
There was a problem hiding this comment.
issue (bug_risk): Avoid bare except
Catching all exceptions with a bare except: can hide unexpected errors. Consider catching specific exceptions or using except Exception:.
| await message.delete() | ||
|
|
||
| if not download.queued: | ||
| await sync_to_async(client.torrents_resume, torrent_hashes=torrent_hash) |
There was a problem hiding this comment.
suggestion: Error handling for torrents_resume
Consider adding error handling for the torrents_resume call to manage potential failures gracefully.
| await sync_to_async(client.torrents_resume, torrent_hashes=torrent_hash) | |
| try: | |
| await sync_to_async(client.torrents_resume, torrent_hashes=torrent_hash) | |
| except Exception as e: | |
| # Handle the error, e.g., log it or notify the user | |
| print(f"Failed to resume torrent: {e}") |
| @@ -1043,7 +1043,7 @@ def hubdrive(url): | |||
| gd_data = soup.select('a[class="btn btn-primary btn-user"]') | |||
| gd_link = gd_data[0]['href'] | |||
| return gd_link | |||
| except Exception as e: | |||
There was a problem hiding this comment.
suggestion: Removed exception variable
Removing the exception variable e might make debugging harder. Consider keeping it for logging purposes.
| except Exception as e: | |
| except Exception as e: | |
| raise DirectDownloadLinkException(f'ERROR: Download link not found try again: {e}') |
| @@ -82,7 +82,7 @@ async def execute_code(func, message): | |||
| try: | |||
| with redirect_stdout(stdout): | |||
| func_return = await func() | |||
| except Exception as e: | |||
There was a problem hiding this comment.
suggestion: Removed exception variable
Removing the exception variable e might make debugging harder. Consider keeping it for logging purposes.
| except Exception as e: | |
| except Exception as e: | |
| raise DirectDownloadLinkException(f'ERROR: Download link not found try again: {e}') |
| @@ -25,7 +25,7 @@ def get_speedtest_results(): | |||
| await editMessage(speed, "Speedtest failed to complete.") | |||
| return | |||
|
|
|||
| string_speed = f"<b>SPEEDTEST INFO</b>\n\n" | |||
There was a problem hiding this comment.
question (bug_risk): Removed f-string
The f-string was removed, but if any variables were intended to be included in the string, this change might cause issues.
| except Exception: | ||
| raise DirectDownloadLinkException('ERROR: Download link not found try again') |
There was a problem hiding this comment.
suggestion (code-quality): Explicitly raise from a previous error (raise-from-previous-error)
| except Exception: | |
| raise DirectDownloadLinkException('ERROR: Download link not found try again') | |
| except Exception as e: | |
| raise DirectDownloadLinkException( | |
| 'ERROR: Download link not found try again' | |
| ) from e |
bot/modules/torrent_select.py
Outdated
| try: | ||
| await aioremove(f['path']) | ||
| await aioremove(file_path) | ||
| except: | ||
| pass |
There was a problem hiding this comment.
issue (code-quality): Use except Exception: rather than bare except: (do-not-use-bare-except)
bot/modules/torrent_select.py
Outdated
| try: | ||
| await aioremove(file['path']) | ||
| except: | ||
| pass |
There was a problem hiding this comment.
issue (code-quality): Use except Exception: rather than bare except: (do-not-use-bare-except)
Summary by Sourcery
This pull request includes significant refactoring to improve code readability and maintainability, removal of unused imports, and the addition of a new CI workflow for automatic code formatting using Ruff.
torrent_select.pyto improve code readability and maintainability by splitting large functions into smaller ones.ruff.yml) to automatically format code using Ruff on push to thedevbranch.