Optimize view_engineer to use DB-side aggregation and cut load time
#12606
Conversation
🔴 Risk threshold exceeded.This pull request contains sensitive edits to multiple files including dojo/utils.py and dojo/metrics/views.py, which may require additional review or configuration in the .dryrunsecurity.yaml file to approve the changes.
🔴 Configured Codepaths Edit in
|
| Vulnerability | Configured Codepaths Edit |
|---|---|
| Description | Sensitive edits detected for this file. Sensitive file paths and allowed authors can be configured in .dryrunsecurity.yaml. |
🔴 Configured Codepaths Edit in dojo/metrics/views.py
| Vulnerability | Configured Codepaths Edit |
|---|---|
| Description | Sensitive edits detected for this file. Sensitive file paths and allowed authors can be configured in .dryrunsecurity.yaml. |
🔴 Configured Codepaths Edit in dojo/metrics/views.py
| Vulnerability | Configured Codepaths Edit |
|---|---|
| Description | Sensitive edits detected for this file. Sensitive file paths and allowed authors can be configured in .dryrunsecurity.yaml. |
We've notified @mtesauro.
All finding details can be found in the DryRun Security Dashboard.
|
Thanks again for a nice optimization PR. If the queries are now much faster, the caching can probably be removed. Since these changes are a little more involved. Is there anyway we can verify the results of the queries are still the same/correct. Would it be feasible to add a little unit test similar to https://github.com/DefectDojo/django-DefectDojo/blob/bugfix/unittests/test_metrics_queries.py ? |
|
Something I also wondered if there is a better/clearer name to the |
There was a problem hiding this comment.
Nice work!
I also agree with @valentijnscholten about tests and variable names if possible.
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
|
@DenysMoskalenko Thank you again for your contributions. I've moved this to the 2.49.0 while we wait for conflicts resolution. |
Thanks, I will fix the conflicts. |
- Remove Cache - Rename "stuff" variables - Change the date range
DenysMoskalenko
force-pushed
the
optimize_metrics_views
branch
from
88a4bb7 to
fc656a2
Compare
|
Conflicts have been resolved. A maintainer will review the pull request shortly. |
There was a problem hiding this comment.
@DenysMoskalenko great work here! I'm approving for now, but happy to wait on merging until you're happy with it 😄
✨ Key Points
view_engineerCOUNT/CASE/Subquery_age_buckets,_product_stats,_augment_series_with_accepted@cache_page(But now it can be removed and I guess - should be removed)select_related&prefetch_related🚀 Performance on our dataset
I cannot run it on the our production dataset for now, but I excpect huge improvement
Screenshots