izimobil / django-rest-framework-datatables

Seamless integration between Django REST framework and Datatables.
http://django-rest-framework-datatables.readthedocs.io/en/latest/
MIT License
395 stars 89 forks source link

add conditional check before adding counts #138

Closed matthewhegarty closed 1 year ago

matthewhegarty commented 1 year ago

With large datasets, avoiding the count check results in a large performance gain (see #122).

This PR adds a conditional check to avoid adding counts. It depends on a new REST_FRAMEWORK_DATATABLES dict in settings.py

The default is to include the count so it won't break any existing implementations.

David - if you are ok with this approach I will add tests and documentation.

codecov-commenter commented 1 year ago

Codecov Report

Merging #138 (4d13d14) into master (433457f) will not change coverage. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##            master      #138    +/-   ##
==========================================
  Coverage   100.00%   100.00%            
==========================================
  Files            8         6     -2     
  Lines          417       307   -110     
==========================================
- Hits           417       307   -110     
Impacted Files Coverage Δ
...st_framework_datatables/django_filters/backends.py 100.00% <100.00%> (ø)
rest_framework_datatables/__init__.py
rest_framework_datatables/filters.py

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

matthewhegarty commented 1 year ago

I've been testing this afternoon and I'm going to close the PR because I find that optimising the count has only a very small effect.

The optimisation is to prevent the initial call to count(). The effectiveness of enabling the flag will depend on how expensive that call is. If it takes several seconds then it is worth enabling the flag if you don't need the 'filtered from N total entries' text. However in the majority of cases I don't think this will be of benefit.

disable_count_tests.txt