Opened 3 months ago

Closed 3 months ago

Last modified 3 months 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 Changed 3 months ago by Ali Toosi

Description: modified (diff)
Has patch: set
Last edited 3 months ago by Mariusz Felisiak (previous) (diff)

comment:2 Changed 3 months ago by Mariusz Felisiak

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 3 months ago by Mariusz Felisiak (previous) (diff)

comment:3 Changed 3 months ago by Mariusz Felisiak

Triage Stage: UnreviewedAccepted

comment:4 in reply to:  2 Changed 3 months ago by Ali Toosi

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

comment:5 Changed 3 months ago by Mariusz Felisiak

Patch needs improvement: unset
Triage Stage: AcceptedReady for checkin

comment:6 Changed 3 months ago by Mariusz Felisiak <felisiak.mariusz@…>

Resolution: fixed
Status: assignedclosed

In faab9e67:

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

comment:7 Changed 3 months ago by Mariusz Felisiak <felisiak.mariusz@…>

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