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

Resolve #494: Reduce duplicate and unnecessary database requests #853

Merged
merged 1 commit into from
Oct 20, 2016

Conversation

seav
Copy link
Contributor

@seav seav commented Oct 20, 2016

Proposed changes in this pull request

  • Resolve Weed out duplicate database requests in templates #494:
    • Use aggregation and annotation (select_related(), prefetch_related(), and annotate()) to combine a list query and the subsequent individual list item queries into just 1 database access whenever possible. (The attachment count of resources is not optimized because it seems aggregation is impossible (?) due to ContentObject’s generic relations.)
    • Use only() or defer() to only retrieve relevant fields or to avoid retrieving the attributes field when it is not needed. Also add get_queryset() managers for the SpatialUnit, Party, and TenureRelationship to defer the models’ attributes field by default. Note: SpatialRelationshipManager was moved from models.py to a new managers.py file.
    • Add simple caching when objects, projects, or organizations are first retrieved. Add the CacheObjectMixin for views that do not have an explicit get_object() method. And switch a single-object view from using get_queryset() to instead declaring the model and leveraging CacheObjectMixin.
    • Improve SpatialQuerySetMixin.get_queryset() to either return all project locations or all excluding the current location for DRY purposes.
    • Update the is_administrator() methods of OrgAdminCheckMixin and ProjectAdminCheckMixin:
      • to inherit from SuperUserCheckMixin instead of having the superuser check code again.
      • to check that the user is an org/project administrator by trying to search for their OA or PM roles in OrganizationRole and ProjectRole instead of constructing a list of org admins or project admins and then checking if the user is in the list.
    • Leverage SuperUserCheckMixin, OrgAdminCheckMixin, and ProjectAdminCheckMixin as needed instead of having repeated code.
    • Remove redundant get_context_data() declarations or code if they can be inherited from a mixin.
    • Simplify reverse() calls in views to reuse already available URL kwargs instead of retrieving organization and project objects.
    • Update templates to use already existing and new context variables.
    • Update unit tests as needed. Special note: Because of the deferred fields, some tests were switched from using refresh_from_db() to retrieving a fresh instance from the database.
  • Other cleanup and improvements:
    • Remove a redundant conditional in a template.
    • Switch around the username and full name of the resource contributor in the resource widget.
    • Switched some Python 2-style super() calls to Python 3.

Limitations

  • The API was left untouched (unless they used common mixins). This should not be a big issue because the API is generally conservative with database requests by design.
  • Duplicate database requests that are traced as being issued inside django-tutelary or django-jsonattrs are left alone.
  • See the limitation above regarding the attachment count of resources. I would recommend ditching this count as this will slow down the website when we have tons of resources.

When should this PR be merged

For Sprint 10.

Risks

None foreseen. The updated code should be functionally the same to the current code (except for the username/full name switch-around in the resources widget).

Follow up actions

None. (But, resolving #572 can further cut down on database requests but that issue needs discussion.)

@seav seav added this to the Sprint 10 milestone Oct 20, 2016
@ian-ross ian-ross merged commit d588c7f into master Oct 20, 2016
@ian-ross ian-ross deleted the debt/dupe-db-requests branch October 20, 2016 09:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants