[SECURITY] Automated DB backup/restore + data corruption detection - closes #62 #63#82
Conversation
sjackson0109
left a comment
There was a problem hiding this comment.
Another outstanding contribution from Ibrahim.
Local CI Simulation ResultsRan all workflow steps locally against this branch prior to owner approval. Results: Code Quality & Testing
Unit Test Results
CI/CD Pipeline
|
pt_backup.py: DatabaseBackupManager with SHA-256 verified SQLite backups, WAL checkpoint, PRAGMA integrity_check, point-in-time restore, retention pruning, and background scheduler. pt_validation.py: DataIntegrityValidator with NaN/Inf detection, OHLCV cross-field consistency checks, Z-score price spike detection, checksum verification, and batch integrity scanning. - 35 unit tests, all passing
d569b0f to
59b1284
Compare
|
@Ibrahim-3d can you review the pipeline failures and refactor to re-try submission? |
Black check failed in CI due to rebase conflict resolution leaving pt_validation.py unformatted. Auto-formatted with Black.
|
Pipeline failure investigated and fixed. The CI job was failing on Black formatting — specifically (the file that had a merge conflict during the rebase onto sjackson0109's Black-reformatted main). The conflict was resolved correctly for logic but the file was left unformatted. Fixed: ran Black on and committed. All other 97 files were already compliant — Black reported '1 file would be reformatted, 97 files would be left unchanged'. |
Manual Testing Guide — PR #82: Automated DB Backup/Restore + Corruption DetectionPrerequisitesgit fetch fork && git checkout feat/62-63-database-backup-validation
cd app1. Run unit testspython -m pytest test_backup_validation.py -vExpected: All 35 tests pass. 2. CI Black formatting check (this was the pipeline failure)uvx black --check app/pt_validation.py
# or: black --check app/pt_validation.pyExpected: 3. Smoke test — backup and restorepython -c "
import tempfile, os, sqlite3
from pt_backup_validation import DatabaseBackupManager # adjust import if needed
with tempfile.TemporaryDirectory() as d:
db = os.path.join(d, 'trade.db')
backup = os.path.join(d, 'trade.db.bak')
conn = sqlite3.connect(db)
conn.execute('CREATE TABLE orders (id INTEGER PRIMARY KEY, symbol TEXT)')
conn.execute(\"INSERT INTO orders VALUES (1, 'BTC-USD')\")
conn.commit()
conn.close()
print('DB created. Run backup + restore manually via BackupManager.')
print('Verify: backup file exists, restore reproduces the row.')
"4. Verify data integrity checksum detects corruptionRefer to Rollbackgit checkout main -- app/pt_validation.py |
|
/gemini review |
|
Hold this one for me please - I want to walk back some unintended changes first. While building the new validator I also stripped docstrings and inline comments off the existing InputValidator methods, which wasn't part of the issue scope and just adds noise to the review. Going to restore those and keep this PR limited to the actual backup + integrity work. Back shortly. |
PR82 review feedback: the prior commit removed docstrings and inline comments from existing InputValidator methods that were unrelated to the data-integrity additions. Rebuild pt_validation.py from main and re-apply only the issue-scoped additions (DataCorruptionError, DataIntegrityValidator, hashlib/math/Tuple imports, module docstring update).
|
Done. I rebuilt pt_validation.py from main so every original docstring and inline comment in InputValidator is back the way it was, then re-applied only the additions this issue actually needs (DataCorruptionError, DataIntegrityValidator, plus the hashlib/math/Tuple imports it needs). Diff is now focused on just the new stuff. 35 tests passing. |
|
@copilot review |
Summary
pt_backup.py:DatabaseBackupManagerwith SHA-256 verified SQLite backups, WAL checkpoint before backup,PRAGMA integrity_check, atomic point-in-time restore, configurable retention pruning, and background schedulerpt_validation.py:DataIntegrityValidatoradded — NaN/Inf scanning, OHLCV cross-field consistency (high≥low, close in range), Z-score price spike detection, SHA-256 data checksums, batch integrity scanningChanges
app/pt_backup.pyapp/pt_validation.py—DataIntegrityValidatorclass added (no breaking changes)app/test_backup_validation.py— 35 testsUsage
Test plan
Closes #62
Closes #63