fix: migrate startup event to lifespan context manager - Fixes #306 - #344
Conversation
|
🎉 Thank you @Tirthpanchori for submitting a Pull Request! We're excited to review your contribution. ✅ Before Review
⚡ Want faster reviews and contributor support? Join our Discord community: 🔗 https://discord.gg/FcXuyw2Rs Maintainers and mentors are active there and can help resolve blockers quickly. Happy Contributing! 🚀 |
|
hey @arpit2006 @ionfwsrijan if everything is ok can you merge this pr before 12, it'll get counted in my score for SSoC. Thank you |
|
@Tirthpanchori , Fix CI fails |
|
@arpit2006 the |
arpit2006
left a comment
There was a problem hiding this comment.
LGTM! 🚀
Review Summary
- Lifespan Migration: Clean replacement of the deprecated
@app.on_event("startup")handler withasynccontextmanagerlifespan(app: FastAPI). - Order & Scope:
lifespanis defined prior toFastAPI(...)initialization and passed vialifespan=lifespan. Diff is clean and tightly scoped tobackend/app/main.py. - Functionality Preserved:
init_db()and_cleanup_active_scans_loop()are properly executed on startup.
@ionfwsrijan , Ready to Merge!
Linked issue
Closes #306
What this PR does
Migrates the FastAPI app's startup logic from the deprecated
@app.on_event("startup")decorator to thelifespanasync context manager pattern, removing theDeprecationWarningemitted at server start.Type of change
ML tier (if applicable)
Stack affected
Changes
Backend
@app.on_event("startup")with anasynccontextmanager-basedlifespanfunction.init_db()and the background_cleanup_active_scans_loop()task now run insidelifespan, unchanged in behaviour.FastAPI(...)constructor now takeslifespan=lifespan.Frontend
New dependencies
Database / schema changes
Testing
How did you test this?
python -m pytest tests/ -vbefore the change: 109 passed,on_eventdeprecation warning present in the summary.on_eventwarning gone.uvicorn app.main:app --reload --port 8000and confirmed no deprecation warning in console output, and/healthresponded correctly.Checklist
console.erroror unhandled Python exceptions introducedrequirements.txt/package.jsonupdated if new dependencies added.pkl,.pt, etc.) are gitignored, not committedAnything reviewers should focus on
Confirm the
lifespanfunction is defined beforeapp = FastAPI(...)(required since it's referenced in the constructor), and thatinit_db()+ the cleanup task still fire correctly on a real server start, not just under test.Screenshots (if UI changed)
N/A (backend-only, no UI change)