cleaner fix delete from db#2573
Conversation
There was a problem hiding this comment.
Hello @doomedraven, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
Summary of Changes
This pull request focuses on cleaning up the database deletion process within the Cuckoo analysis framework. It introduces a dedicated delete_tasks function in lib/cuckoo/core/database.py to handle task deletions based on various criteria, and replaces direct calls to db.list_tasks with db.delete_tasks in lib/cuckoo/common/cleaners_utils.py. This change aims to provide a more streamlined and maintainable approach to removing tasks from the database.
Highlights
- Database Deletion Function: A new
delete_tasksfunction is added tolib/cuckoo/core/database.pyto centralize and simplify task deletion logic. - Replacement of list_tasks with delete_tasks: Instances of
db.list_taskswith thedelete=Trueparameter inlib/cuckoo/common/cleaners_utils.pyare replaced with calls to the newdb.delete_tasksfunction. - Parameter Adjustment in delete_bulk_tasks_n_folders: The
cuckoo_clean_lower_scorefunction now callsdelete_bulk_tasks_n_folderswith thedelete_db_tasks=Trueparameter. - Test Update: The test
test_delete_tasksintests/test_database.pyis updated to use thetask_idsparameter.
Changelog
Click here to see the changelog
- lib/cuckoo/common/cleaners_utils.py
- Replaced
db.list_tasks(delete=True)withdb.delete_tasksincuckoo_clean_failed_tasks,cuckoo_clean_before,cuckoo_clean_pending_tasks, andcuckoo_clean_range_tasks. - Added
delete_db_tasks=Trueto thedelete_bulk_tasks_n_folderscall withincuckoo_clean_lower_score. - Removed an extraneous comma in the imports.
- Replaced
- lib/cuckoo/core/database.py
- Removed the
deleteparameter from thelist_tasksfunction. - Added a new
delete_tasksfunction to handle task deletions based on various criteria. - Added a new
delete_taskfunction to delete a single task by ID. - Removed the
delete_tasksfunction that took a list of IDs as input.
- Removed the
- tests/test_database.py
- Updated the
test_delete_taskstest case to use thetask_idsparameter when callingdb.delete_tasks.
- Updated the
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
A cleaner database,
A task deleted with grace,
Code's clarity shines.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
The pull request aims to improve the task deletion process by introducing a dedicated delete_tasks function in lib/cuckoo/core/database.py and utilizing it across various cleaner utilities. This change streamlines the code and enhances maintainability by centralizing the task deletion logic. The changes look good overall, but I have a few suggestions for improvement.
Summary of Findings
- Duplication of logic in
delete_tasks: Thedelete_tasksfunction inlib/cuckoo/core/database.pymirrors much of the filtering logic present in thelist_tasksfunction. This duplication could lead to inconsistencies and increased maintenance overhead. Consider refactoring to reuse the filtering logic. - Missing error handling in
delete_bulk_tasks_n_folders: Thedelete_bulk_tasks_n_foldersfunction inlib/cuckoo/common/cleaners_utils.pyiterates through task IDs and attempts to delete corresponding folders. However, it lacks specific error handling for database deletion failures, potentially leaving orphaned data in the database.
Merge Readiness
The pull request introduces significant improvements to the task deletion process. However, addressing the duplicated filtering logic in delete_tasks and incorporating more robust error handling in delete_bulk_tasks_n_folders would further enhance the code's quality and reliability. I am unable to directly approve this pull request, and recommend that others review and approve this code before merging. Given the medium severity issues identified, I recommend addressing them before merging.
No description provided.