ci: test LLM migration check #9072
Conversation
✅ Deploy Preview for heartex-docs canceled.
|
✅ Deploy Preview for label-studio-storybook canceled.
|
✅ Deploy Preview for label-studio-docs-new-theme canceled.
|
✅ Deploy Preview for label-studio-playground canceled.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #9072 +/- ##
============================================
+ Coverage 39.16% 81.51% +42.35%
============================================
Files 746 271 -475
Lines 58564 24491 -34073
Branches 9365 0 -9365
============================================
- Hits 22939 19965 -2974
+ Misses 35621 4526 -31095
+ Partials 4 0 -4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
nikitabelonogov
left a comment
nikitabelonogov
left a comment
There was a problem hiding this comment.
Migration Review
🚨 Memory Risk: Removing iterate_queryset and using View.objects.all() directly will load all View objects into memory at once, which can cause OOM errors in production with large datasets.
Recommendation: Use View.objects.all().iterator() to process views in chunks, or use View.objects.all().values('id', 'data') with update() if full model instances aren't needed.
|
|
||
| try: | ||
| views = iterate_queryset(View.objects.all()) | ||
| views = View.objects.all() |
There was a problem hiding this comment.
🚨 Memory risk: Using View.objects.all() directly loads all View objects into memory. Use View.objects.all().iterator() to process in chunks.
| logger.info(f'Starting reverse migration {migration_name}') | ||
|
|
||
| views = iterate_queryset(View.objects.all()) | ||
| views = View.objects.all() |
There was a problem hiding this comment.
🚨 Same memory risk in reverse migration. Use View.objects.all().iterator() instead.
nikitabelonogov
left a comment
nikitabelonogov
left a comment
There was a problem hiding this comment.
Summary
🚨 Critical Memory Issue: Removing iterate_queryset reintroduces OOM risk by loading all View objects into memory. Both forward and reverse migrations should use View.objects.all().iterator() to process in chunks.
nikitabelonogov
force-pushed
the
revert-9028-fb-utc-444-oom-fix
branch
from
cb3acac to
f16fac4
Compare
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Migration Review
Recommendation: Add error handling and status tracking consistent with .
There was a problem hiding this comment.
Migration Review
Recommendation: Add error handling and status tracking consistent with forward_migration_job.
nikitabelonogov
force-pushed
the
revert-9028-fb-utc-444-oom-fix
branch
from
f16fac4 to
05b2287
Compare
|
|
||
| try: | ||
| views = iterate_queryset(View.objects.all()) | ||
| views = View.objects.all() |
There was a problem hiding this comment.
⚡ Performance: Replacing iterate_queryset() with .all() loads all View objects into memory at once, which can cause memory issues or timeouts in production databases with many views. Use iterate_queryset() for safe chunked iteration.
| logger.info(f'Starting reverse migration {migration_name}') | ||
|
|
||
| views = iterate_queryset(View.objects.all()) | ||
| views = View.objects.all() |
There was a problem hiding this comment.
⚡ Performance: Same issue as forward migration - use iterate_queryset() instead of .all() to handle large querysets safely.
DO NOT MERGE