annashamray commented 5 years ago

Reason: Every time Audittrail resource is requested for some object full scan of the table happens

codecov-io commented 5 years ago

Codecov Report

Merging #102 into master will increase coverage by 1.11%. The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #102      +/-   ##
==========================================
+ Coverage   32.14%   33.26%   +1.11%     
==========================================
  Files          62       62              
  Lines        2252     2315      +63     
  Branches      331      349      +18     
==========================================
+ Hits          724      770      +46     
- Misses       1504     1518      +14     
- Partials       24       27       +3

Impacted Files	Coverage Δ
vng_api_common/audittrails/models.py	`100% <100%> (ø)`	:arrow_up:
vng_api_common/caching/signals.py	`81.81% <0%> (-3.9%)`	:arrow_down:
vng_api_common/conf/api.py	`100% <0%> (ø)`	:arrow_up:
vng_api_common/inspectors/fields.py	`34.42% <0%> (+0.33%)`	:arrow_up:
vng_api_common/caching/etags.py	`95.45% <0%> (+1.45%)`	:arrow_up:
vng_api_common/authorizations/validators.py	`75% <0%> (+2.5%)`	:arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update d449a05...d1985b6. Read the comment docs.

annashamray commented 5 years ago

Can we work https://stackoverflow.com/a/51880653 into the solution instead of defining custom SQL?

I don't like not having that index defined on the model and not uninstalling the index when reverse-migrating.

@sergei-maertens I tried to go this way - you can see it in the https://github.com/VNG-Realisatie/vng-api-common/pull/102/commits/2c0d8d0cf7c980feea5086469077c9f9d550cff2 commit, but it didn't work for Django==2.1

sergei-maertens commented 5 years ago

@annashamray all APIs should be on Django 2.2 by now (I upgraded them a while ago), so we can drop support for Django 2.0 and 2.1 :+1:

annashamray commented 5 years ago

@annashamray all APIs should be on Django 2.2 by now (I upgraded them a while ago), so we can drop support for Django 2.0 and 2.1

Great news! I will return to https://github.com/VNG-Realisatie/vng-api-common/pull/102/commits/6514419098273baf50a2959654296ba994b9df1d commit then. I think it is the cleanest way to create this index in Django 2.2

joeribekker commented 5 years ago

I want this in for 1.0 APIs as well as future versions.

sergei-maertens commented 5 years ago

Rebased so that backporting is easier

VNG-Realisatie / vng-api-common

add index to hoofd_object field of Audittrail #102

Codecov Report