Update test instances to use local commitid and branch fields #426
Conversation
Signed-off-by: joseph-sentry <[email protected]>
This is a task meant to populate the commitid and branch fields for test instances that are missing this information. Signed-off-by: joseph-sentry <[email protected]>
🔍 Existing Issues For ReviewYour pull request is modifying functions with the following pre-existing issues: 📄 File: tasks/test_results_processor.py
Did you find this useful? React with a 👍 or 👎 |
Codecov ReportAttention: Patch coverage is
✅ All tests successful. No failed tests found Additional details and impacted files@@ Coverage Diff @@
## main #426 +/- ##
==========================================
- Coverage 97.44% 97.37% -0.07%
==========================================
Files 395 396 +1
Lines 33426 33454 +28
==========================================
+ Hits 32572 32576 +4
- Misses 854 878 +24
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Codecov ReportAttention: Patch coverage is
✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## main #426 +/- ##
==========================================
- Coverage 97.47% 97.41% -0.06%
==========================================
Files 426 427 +1
Lines 34117 34257 +140
==========================================
+ Hits 33254 33371 +117
- Misses 863 886 +23
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 1 file with indirect coverage changes This change has been scanned for critical changes. Learn more |
Codecov ReportAttention: Patch coverage is
✅ All tests successful. No failed tests found @@ Coverage Diff @@
## main #426 +/- ##
==========================================
- Coverage 97.44% 97.37% -0.07%
==========================================
Files 395 396 +1
Lines 33426 33454 +28
==========================================
+ Hits 32572 32576 +4
- Misses 854 878 +24
Flags with carried forward coverage won't be shown. Click here to find out more.
|
| commitid=None, | ||
| ) | ||
| num_test_instances = test_instance_filter.count() | ||
| all_test_instances = test_instance_filter.all() |
There was a problem hiding this comment.
Depending on how many test instances we have this could lead to OOM errors, right? I think there are millions of test instances, after all.
Consider using the yield_per here, so we don't get everything into memory at once.
There was a problem hiding this comment.
You can use the yield_per value as the chunk size and simplify the code a bit.
There was a problem hiding this comment.
this is using the django models so because we're turning it into chunks later in the code, this won't load everything at once, it should only load that chunk of objects, update them, then load the next chunk
There was a problem hiding this comment.
Interesting. Today I learned.
We want to move the commitid and branch information for a test instance to directly on the object, so that when we query we don't have to join with the commits table.