Auto optimizations #7

evenicoulddoit · 2017-04-16T23:14:48Z

TODOS:

Rebase / rationalise
Add settings to default optimize on/off
Document

* Drops Django 1.9, adds Django 1.11 * 1.8 remains due to being LTS * Drops DRF 3.3, adds DRF 3.6

* Which can be used in place of the explicit mixin approach if desired

* Depending on what is being expanded * By default is disabled, can easily be enabled globally/per view * Tested to support optimizing calls to both single related instances, and to related querysets. Uses `select_related` and `prefetch_related` as required * Can be provided explicitly on non-model serializers * Can be disabled for individual expandable relations

codecov-io · 2017-04-17T13:28:06Z

Codecov Report

❗ No coverage uploaded for pull request base (master@7660bbd). Click here to learn what that means.
The diff coverage is 100%.

@@            Coverage Diff            @@
##             master       #7   +/-   ##
=========================================
  Coverage          ?   99.77%           
=========================================
  Files             ?        5           
  Lines             ?      450           
  Branches          ?        0           
=========================================
  Hits              ?      449           
  Misses            ?        1           
  Partials          ?        0

Impacted Files	Coverage Δ
...est_framework_serializer_extensions/serializers.py	`100% <100%> (ø)`
rest_framework_serializer_extensions/views.py	`98.24% <100%> (ø)`

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 7660bbd...45c4db3. Read the comment docs.

evenicoulddoit · 2017-04-17T16:15:07Z

Fixes #6 as an aside

* Remove the requirement to add to `INSTALLED_APPS`, unnecessary * Better explanation of `QUERY_PARAMS_ENABLED` setting

evenicoulddoit · 2017-04-17T17:55:28Z

@gareth-lloyd would love your input on this if you can spare the time? :)

gareth-lloyd · 2017-05-01T13:18:36Z

I read through this today. I like the approach! I'll try to find time for a line-by-line review and then I'd be happy to start using it on some of our views ASAP.

jojolalpin · 2018-01-02T13:17:30Z

Hi,
Anything I can do to help merge the auto optimization ?
CI only fails for django 1.8 and DRF 3.6, but DRF is now at 3.7.7 and oldest supported version of Django is 1.10.

KyeRussell · 2018-01-02T13:21:50Z

As discussed elsewhere, Django 1.8 support should not be dropped as per Django’s support schedule.

jojolalpin · 2018-01-02T14:34:37Z

So maybe make a try with DRF 3.6.4 (last 3.6 version) instead of 3.6.2

This reverts commit 68dc15e.

evenicoulddoit · 2018-01-03T20:51:42Z

@jojolalpin @KyeRussell the tests pass now on this branch, and master has been merged into it, so it conforms to the current Django support lifecycles as well as Rest framework current - 2. The only thing preventing it from being merged is lack of feedback. I could just merge it as-is, as it shouldn't negatively affect users unless they turn the feature on. Happy to take suggestions!

jojolalpin · 2018-01-04T16:01:10Z

Hi, Yesterday I managed to pass some tests on my fork but was struggling with the set thing for Django 2. I will make tries with the updated branch as I have a project with a set of interlinked serializers that should show the gains in perf (as for now it takes 30 secs to run a query...) I will let you know asap the results and any problems I encounter. EDIT: It works like a charm. THANKS A LOT ! 1 thing, with a ManyToMany field using a through model (a person, an organization and a membership, organization.members is a ManyToManyField to Person Through Memberships. I can expand members in organization but not members__person But I can expand memberships__person so this is not blocking and maybe even just a limitation of the ManyToManyField. Performance: now it runs in 3 seconds instead of 30 with few queries. Before I had already declared primarykey fields in the serializers that I commented so I cannot measure exacly the gain). Le mer. 3 janv. 2018 à 21:51, Ian Clark <[email protected]> a écrit :

…

@jojolalpin <https://github.com/jojolalpin> @KyeRussell <https://github.com/kyerussell> the *tests pass* now on this branch, and master has been merged into it, so it conforms to the current Django support lifecycles as well as Rest framework current - 2. The only thing preventing it from being merged is lack of feedback. I could just merge it as-is, as it shouldn't negatively affect users unless they turn the feature on. Happy to take suggestions! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#7 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACrozx9sLo6-7ydGhxoKCi_gM5DH8j3Oks5tG-hegaJpZM4M-we0> .

jojolalpin · 2018-07-02T17:07:57Z

Hi, any idea when you merge this ?

evenicoulddoit · 2018-07-02T17:47:13Z

Hey @jojolalpin. Sorry, I'm no longer working with DRF so I've not had the ability to run this in the wild. What's your experience been with it? Happy to build a very though I think, because IIRC I've turned the feature off by default?

jojolalpin · 2018-07-02T17:52:23Z

Hi Ian, I pull from the commit with this option. Everything works well and I think you should merge what is ok if you do not work on it anymore. Your extension comes in my default setting with drf. Thanks a lot for it. Julien Le lun. 2 juil. 2018 à 19:47, Ian Clark <[email protected]> a écrit :

…

Hey @jojolalpin <https://github.com/jojolalpin>. Sorry, I'm no longer working with DRF so I've not had the ability to run this in the wild. What's your experience been with it? Happy to build a very though I think, because IIRC I've turned the feature off by default? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#7 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACrozyM21kGUsnPOEYsJe3_34sTIZ0QRks5uClyigaJpZM4M-we0> .

jojolalpin · 2018-07-02T18:17:56Z

Thanks a lot! Le lun. 2 juil. 2018 à 20:01, Ian Clark <[email protected]> a écrit :

…

Merged #7 <#7> . — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#7 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACrozzjVLflCSelWXyO7odF_AxrsgV_Zks5uCl_1gaJpZM4M-we0> .

evenicoulddoit · 2018-07-02T18:52:00Z

@jojolalpin now available in 0.6.0 - thanks for the extreme patience!

jojolalpin · 2018-07-02T18:53:20Z

Just a quick question, permissions are still not checked when you expand? Le lun. 2 juil. 2018 à 20:52, Ian Clark <[email protected]> a écrit :

…

@jojolalpin <https://github.com/jojolalpin> now available in 0.6.0 - thanks for the extreme patience! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#7 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACroz4rvMenHkVY3e8EVgVlVKHNzhPcKks5uCmvRgaJpZM4M-we0> .

evenicoulddoit · 2018-07-02T19:01:32Z

I think that's correct, but this was all done so long ago! Would need to sit down properly to give you a more complete answer

jojolalpin · 2018-07-02T19:03:44Z

If you did not add this "recently", permissions are not checked. I had to make a filter working with a not so old commit. Le lun. 2 juil. 2018 à 21:01, Ian Clark <[email protected]> a écrit :

…

I think that's correct, but this was all done so long ago! Would need to sit down properly to give you a more complete answer — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#7 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACroz80e7NO_6E5Nmk7Lf0jtr5qxeYi-ks5uCm4NgaJpZM4M-we0> .

evenicoulddoit · 2018-07-02T19:10:02Z

Happy to take contributions if you've worked on something?

jojolalpin · 2018-07-02T19:50:07Z

No it was very specific for a rarely called query and very basic. Could have done more beautiful but I was lazy. Le lun. 2 juil. 2018 à 21:10, Ian Clark <[email protected]> a écrit :

…

Happy to take contributions if you've worked on something? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#7 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACrozw_uhx8p-Jl_UZN7ALoXm3TXbPC7ks5uCnALgaJpZM4M-we0> .

jojolalpin · 2018-07-02T22:07:30Z

to end about permissions, having each object read permissions (I use dry-rest-permissions <https://github.com/dbkaplan/dry-rest-permissions>) in the response allows to simply filter it. Le lun. 2 juil. 2018 à 21:49, jojolalpin <[email protected]> a écrit :

…

No it was very specific for a rarely called query and very basic. Could have done more beautiful but I was lazy. Le lun. 2 juil. 2018 à 21:10, Ian Clark ***@***.***> a écrit : > Happy to take contributions if you've worked on something? > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub > <#7 (comment)>, > or mute the thread > <https://github.com/notifications/unsubscribe-auth/ACrozw_uhx8p-Jl_UZN7ALoXm3TXbPC7ks5uCnALgaJpZM4M-we0> > . >

evenicoulddoit added 2 commits April 17, 2017 10:37

Bump: Django and Rest Framework support

68dc15e

* Drops Django 1.9, adds Django 1.11 * 1.8 remains due to being LTS * Drops DRF 3.3, adds DRF 3.6

Add: Complete extensions model/non-model serializers

9e2dedb

* Which can be used in place of the explicit mixin approach if desired

evenicoulddoit force-pushed the auto-optimizations branch from fb8e5e5 to 34ab554 Compare April 17, 2017 13:21

evenicoulddoit force-pushed the auto-optimizations branch from 34ab554 to 72a8335 Compare April 17, 2017 13:27

Document: Auto optimization feature. Other minor fixes.

006e3ee

* Remove the requirement to add to `INSTALLED_APPS`, unnecessary * Better explanation of `QUERY_PARAMS_ENABLED` setting

evenicoulddoit mentioned this pull request Aug 15, 2017

Make utils.local_import check INSTALLED_APPS for compound paths #14

Merged

Merge branch 'master' into auto-optimizations

a5e1dd2

evenicoulddoit added 3 commits January 3, 2018 19:50

Revert "Bump: Django and Rest Framework support"

fb3aea9

This reverts commit 68dc15e.

Merge branch 'master' into auto-optimizations

0301adb

Fix: Add explicit on_delete for test model

58d7fa1

evenicoulddoit force-pushed the auto-optimizations branch from fe69435 to 58d7fa1 Compare January 3, 2018 20:15

Fix: Use .set() to update reverse many-to-many

45c4db3

evenicoulddoit force-pushed the auto-optimizations branch from 7b70b57 to 45c4db3 Compare January 3, 2018 20:44

evenicoulddoit merged commit b66500a into master Jul 2, 2018

evenicoulddoit deleted the auto-optimizations branch July 2, 2018 18:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Auto optimizations #7

Auto optimizations #7

evenicoulddoit commented Apr 16, 2017 •

edited

Loading

codecov-io commented Apr 17, 2017 •

edited

Loading

evenicoulddoit commented Apr 17, 2017

evenicoulddoit commented Apr 17, 2017

gareth-lloyd commented May 1, 2017

jojolalpin commented Jan 2, 2018

KyeRussell commented Jan 2, 2018

jojolalpin commented Jan 2, 2018

evenicoulddoit commented Jan 3, 2018

jojolalpin commented Jan 4, 2018 via email •

edited

Loading

jojolalpin commented Jul 2, 2018

evenicoulddoit commented Jul 2, 2018

jojolalpin commented Jul 2, 2018 via email

jojolalpin commented Jul 2, 2018 via email

evenicoulddoit commented Jul 2, 2018

jojolalpin commented Jul 2, 2018 via email

evenicoulddoit commented Jul 2, 2018

jojolalpin commented Jul 2, 2018 via email

evenicoulddoit commented Jul 2, 2018

jojolalpin commented Jul 2, 2018 via email

jojolalpin commented Jul 2, 2018 via email

Auto optimizations #7

Auto optimizations #7

Conversation

evenicoulddoit commented Apr 16, 2017 • edited Loading

codecov-io commented Apr 17, 2017 • edited Loading

Codecov Report

evenicoulddoit commented Apr 17, 2017

evenicoulddoit commented Apr 17, 2017

gareth-lloyd commented May 1, 2017

jojolalpin commented Jan 2, 2018

KyeRussell commented Jan 2, 2018

jojolalpin commented Jan 2, 2018

evenicoulddoit commented Jan 3, 2018

jojolalpin commented Jan 4, 2018 via email • edited Loading

jojolalpin commented Jul 2, 2018

evenicoulddoit commented Jul 2, 2018

jojolalpin commented Jul 2, 2018 via email

jojolalpin commented Jul 2, 2018 via email

evenicoulddoit commented Jul 2, 2018

jojolalpin commented Jul 2, 2018 via email

evenicoulddoit commented Jul 2, 2018

jojolalpin commented Jul 2, 2018 via email

evenicoulddoit commented Jul 2, 2018

jojolalpin commented Jul 2, 2018 via email

jojolalpin commented Jul 2, 2018 via email

evenicoulddoit commented Apr 16, 2017 •

edited

Loading

codecov-io commented Apr 17, 2017 •

edited

Loading

jojolalpin commented Jan 4, 2018 via email •

edited

Loading