Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Build line filter fix #8343

Merged

Conversation

SchrodingersGat
Copy link
Member

@SchrodingersGat SchrodingersGat commented Oct 23, 2024

Tracking down some issues with filtering of BuildLine objects via API

Changes

The major change here has been to optimize and simplify the queryset annotations. The generated SQL was getting out of hand - it may have simply been "too complex"?

For a sample dataset I have got the query time down from ~10 seconds to ~500ms (for the same data).

Observations

Using the postgresql backend, the build.test_api.BuildLineTests.test_filter_available method takes > 20 seconds to run. It takes under 1 second with sqlite backend. Not sure what is going on there, even after extended testing.

Strangely the same API calls are now quite "efficient" in actual operation (i.e. not in test mode) - even with postgresql backend.

@SchrodingersGat SchrodingersGat added bug Identifies a bug which needs to be addressed api Relates to the API CI CI / unit testing ecosystem labels Oct 23, 2024
@SchrodingersGat SchrodingersGat added this to the 0.17.0 milestone Oct 23, 2024
Copy link

netlify bot commented Oct 23, 2024

Deploy Preview for inventree-web-pui-preview canceled.

Name Link
🔨 Latest commit ebaf86d
🔍 Latest deploy log https://app.netlify.com/sites/inventree-web-pui-preview/deploys/671b1c2af390640008b79475

Copy link

codecov bot commented Oct 23, 2024

Codecov Report

Attention: Patch coverage is 96.66667% with 1 line in your changes missing coverage. Please review.

Project coverage is 84.07%. Comparing base (556ff4f) to head (ebaf86d).
Report is 7 commits behind head on master.

Files with missing lines Patch % Lines
src/backend/InvenTree/build/api.py 87.50% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #8343      +/-   ##
==========================================
- Coverage   84.30%   84.07%   -0.23%     
==========================================
  Files        1166     1166              
  Lines       52980    53399     +419     
  Branches     1946     1938       -8     
==========================================
+ Hits        44663    44894     +231     
- Misses       7836     8027     +191     
+ Partials      481      478       -3     
Flag Coverage Δ
backend 85.99% <96.66%> (+0.08%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

- Multi-level annotation proves to be very expensive
- Reduce complexity, save a bunch of time on queries
- Remove 'total_allocated_stock' field
- Adjust API query filter
@SchrodingersGat SchrodingersGat merged commit 6be6c4b into inventree:master Oct 25, 2024
26 checks passed
@SchrodingersGat SchrodingersGat deleted the build-line-filter-fix branch October 25, 2024 04:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api Relates to the API bug Identifies a bug which needs to be addressed CI CI / unit testing ecosystem
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant