fix(backend): resolve correct database path in reset_database.py#1378
fix(backend): resolve correct database path in reset_database.py#1378AbiramiR-27 wants to merge 5 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe database reset script now uses the configured database path, removes SQLite companion and test database files, normalizes and deduplicates candidates, reports deletion results, and exits on errors. A frontend destructuring statement receives a syntax-only trailing comma. ChangesDatabase reset behavior
Frontend syntax cleanup
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested labels: Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
backend/reset_database.py (3)
17-17: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueVerify that the path is a directory.
Using
os.path.isdir()is safer thanos.path.exists()to ensure that the path is actually a directory before attempting to construct file paths and search within it.💡 Proposed refactor
- if os.path.exists(db_dir): + if os.path.isdir(db_dir):🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/reset_database.py` at line 17, Update the path check in the database reset flow to use os.path.isdir(db_dir) instead of os.path.exists(db_dir), ensuring subsequent file construction and searching only occur when db_dir is a directory.
33-33: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove redundant
list()conversion.The
list()call is unnecessary becausesorted()accepts any iterable and natively returns a list.💡 Proposed refactor
- unique_db_files = sorted(list(set(os.path.abspath(f) for f in db_files))) + unique_db_files = sorted(set(os.path.abspath(f) for f in db_files))🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/reset_database.py` at line 33, Remove the redundant list conversion in the unique_db_files assignment while preserving the set-based deduplication, absolute-path mapping, and sorted list result.
6-6: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd return type hint.
As per path instructions, ensure proper use of type hints by explicitly indicating that this function returns
None.💡 Proposed refactor
-def delete_db_files(): +def delete_db_files() -> None:🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/reset_database.py` at line 6, Update the delete_db_files function signature to explicitly annotate its return type as None, without changing its behavior or implementation.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/reset_database.py`:
- Line 37: Update the database cleanup condition in the reset logic to use
os.path.isfile(db_file) instead of os.path.exists(db_file), ensuring os.remove()
is only called for matching files and not directories.
---
Nitpick comments:
In `@backend/reset_database.py`:
- Line 17: Update the path check in the database reset flow to use
os.path.isdir(db_dir) instead of os.path.exists(db_dir), ensuring subsequent
file construction and searching only occur when db_dir is a directory.
- Line 33: Remove the redundant list conversion in the unique_db_files
assignment while preserving the set-based deduplication, absolute-path mapping,
and sorted list result.
- Line 6: Update the delete_db_files function signature to explicitly annotate
its return type as None, without changing its behavior or implementation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1937a5c0-c6c0-482f-8a02-ff3aec22553d
📒 Files selected for processing (1)
backend/reset_database.py
There was a problem hiding this comment.
We'll soon be having a scripts/ directory inside the backend/ folder. When that happens we'll be moving this file into that directory.
There was a problem hiding this comment.
Understood, thanks for letting me know!
e8cb013 to
4835cbb
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/reset_database.py`:
- Around line 18-24: Update the pattern handling in the database reset logic to
remove SQLite companion files for .sqlite3 databases as well as the base files.
Extend the patterns used by the reset routine to include .sqlite3-journal,
.sqlite3-wal, and .sqlite3-shm variants, preserving the existing cleanup
behavior for other database extensions.
- Around line 35-48: Update the deletion flow in the reset database script so
any PermissionError or other exception while removing files causes the script to
exit unsuccessfully, rather than only printing the error. Preserve the existing
successful deletion messages and no-files behavior, but propagate or explicitly
raise a failure after deletion errors so automation cannot continue with a stale
database.
- Around line 15-30: Restrict the file patterns in the database cleanup flow to
the configured DATABASE_PATH stem and its journal, WAL, SHM, and sqlite
variants, while retaining only the explicitly supported test database files.
Remove broad "*.db" and "*.sqlite3" globs so unrelated databases in the database
directory cannot be deleted.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3d12118d-392a-45d0-b464-fb45dd6a331d
📒 Files selected for processing (2)
backend/reset_database.pyfrontend/src/components/Media/ZoomableImage.tsx
There was a problem hiding this comment.
🧹 Nitpick comments (1)
backend/reset_database.py (1)
24-25: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSimplify set creation and sorting.
As per path instructions regarding Python best practices, using a set comprehension directly within
sorted()is more idiomatic and concise. Thelist()conversion is unnecessary becausesorted()inherently returns a new list.♻️ Proposed refactor
# Normalize paths and remove duplicates - unique_db_files = sorted(list(set(os.path.abspath(f) for f in db_files))) + unique_db_files = sorted({os.path.abspath(f) for f in db_files})🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/reset_database.py` around lines 24 - 25, Update the unique_db_files assignment to pass a set comprehension directly to sorted(), removing the unnecessary list() conversion while preserving absolute-path normalization, deduplication, and ordering.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@backend/reset_database.py`:
- Around line 24-25: Update the unique_db_files assignment to pass a set
comprehension directly to sorted(), removing the unnecessary list() conversion
while preserving absolute-path normalization, deduplication, and ordering.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 10a73a30-5e99-47e2-bb58-b30060fcb585
📒 Files selected for processing (1)
backend/reset_database.py
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@AbiramiR-27 you can go ahead with the move now! |
… failure in reset_database.py
…reset_database.py
b0a15e0 to
6a116d1
Compare
|
@rohan-pandeyy Done! I've moved the script to |
2 participants
Addressed Issues:
Fixes #1375
Screenshots/Recordings:
N/A (This is a backend utility script update with no user interface changes).
Additional Notes:
Previously, the
backend/reset_database.pyscript was hardcoded to search for database files usingglob.glob("app/database/*.db"). However:DATABASE_PATHinapp/config/settings.py.This meant that running the reset database script did not actually clean or delete the active database.
Changes made in this PR:
DATABASE_PATHfromapp.config.settings.delete_db_files()to explicitly targetDATABASE_PATHalong with any associated SQLite journaling or transaction files (-journal,-wal,-shm)..dbor.sqlite3files.test_db.sqlite3generated during unit tests).ruff) and unit tests (pytest), all of which passed successfully.AI Usage Disclosure:
I have used the following AI models and tools: Gemini 3.5 Flash (Medium) via Antigravity coding assistant.
Checklist
Summary by CodeRabbit