Opened 2 years ago

Closed 2 years ago

Last modified 2 years ago

#33680 closed Bug (fixed)

Documentation example of customising model instance loading has a bug

Reported by: Ali Toosi Owned by: Ali Toosi
Component: Documentation Version: 4.0
Severity: Normal Keywords: documentation, from_db, model instance loading
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description (last modified by Ali Toosi)

The docs provide an example of how the model instance loading can be changed and how _loaded_values can be saved for future comparison here: https://docs.djangoproject.com/en/4.0/ref/models/instances/

In this example, it checks if there are any values not loaded from db then add them as DEFERRED values so class instantiation would work (cls(*values)). However, this would mean values list has items now that do not map to any field_names so when at the end of the function, we zip them together and store in _loaded_values, the code will fail.

The easiest fix for this is to update the field_names too so they would 1. work and 2. show which fields were not loaded from the db at the time.

I've opened a PR for this here: https://github.com/django/django/pull/15664

Change History (7)

comment:1 by Ali Toosi, 2 years ago

Description: modified (diff)
Has patch: set
Version 0, edited 2 years ago by Ali Toosi (next)

comment:2 by Mariusz Felisiak, 2 years ago

Owner: changed from nobody to Ali Toosi
Patch needs improvement: set
Status: newassigned

Thanks for the ticket.

The easiest fix for this is to update the field_names too so they would 1. work and 2. show which fields were not loaded from the db at the time.

I'd keep the previous behavior and rather filter out DEFERRED from the list of values, e.g.

  • docs/ref/models/instances.txt

    diff --git a/docs/ref/models/instances.txt b/docs/ref/models/instances.txt
    index 3638c6ccff..ec0232673e 100644
    a b are loaded from the database::  
    103103        instance._state.adding = False
    104104        instance._state.db = db
    105105        # customization to store the original field values on the instance
    106         instance._loaded_values = dict(zip(field_names, values))
     106        instance._loaded_values = dict(
     107            zip(field_names, (value for value in values if value is not DEFERRED))
     108        )
    107109        return instance
    108110
    109111    def save(self, *args, **kwargs):
Last edited 2 years ago by Mariusz Felisiak (previous) (diff)

comment:3 by Mariusz Felisiak, 2 years ago

Triage Stage: UnreviewedAccepted

in reply to:  2 comment:4 by Ali Toosi, 2 years ago

Replying to Mariusz Felisiak:
Thanks, Mariusz. I've updated the PR with your suggestion.

comment:5 by Mariusz Felisiak, 2 years ago

Patch needs improvement: unset
Triage Stage: AcceptedReady for checkin

comment:6 by Mariusz Felisiak <felisiak.mariusz@…>, 2 years ago

Resolution: fixed
Status: assignedclosed

In faab9e67:

Fixed #33680 -- Corrected example of customizing model loading in docs.

comment:7 by Mariusz Felisiak <felisiak.mariusz@…>, 2 years ago

In 8b2a93ee:

[4.0.x] Fixed #33680 -- Corrected example of customizing model loading in docs.

Backport of faab9e6769b01c18d9e3a31504601452eede6150 from main

Note: See TracTickets for help on using tickets.
Back to Top