#22787 closed New feature (wontfix)

Add a default implementation of Model.natural_key()

Reported by: jian Owned by: jian
Component: Core (Serialization) Version: master
Severity: Normal Keywords:
Cc: Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

In order to use natural key serialization, the user has to define model.natural_key and manager.get_by_natural_key methods for every single model to be serialized. This is tedious and requires many LOC if the user wants to serialize multiple models, especially if those models do not already have custom manager classes.

To remedy this, the linked pull request defines a base class implementation of model.natural_key and manager.get_by_natural_key. This implementation automatically produces natural keys that uniquely identify each model without recourse to the pk field, and the results are qualitatively similar to what the user would have defined in custom methods.

In the rare instance where the model lacks any uniqueness constraints, this approach would not work, and invoking natural_key or get_by_natural_key would raise a NotImplementedError. In such a scenario, it is not clear that even a user-defined natural key would work (but, as always, the user is free to implement one anyway).

Currently, if the user requests a natural key serialization, Django introspects the model using hasattr(model, 'natural_key') and silently falls back to standard serialization if a natural key serialization is not available. Using hasattr is no longer possible with the new implementation, so this pull request removes these checks, and Django will instead complain with an informative message such as CommandError: Unable to serialize database: The Question class must provide a natural_key() method. This might be a backward-incompatible change, but it is the correct one. When a natural key serialization is requested but unavailable, the correct behavior is for Django to complain loudly rather than being silent ("explicit is better than implicit").

The pull request includes a new regression test, and passes all tests using SQLite version 3.7.13.

django-developers discussion: https://groups.google.com/d/msg/django-developers/6ZboknjrSkg/vOQXDSDhG28J

Change History (4)

comment:1 Changed 15 months ago by jian

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

comment:2 Changed 15 months ago by timo

  • Summary changed from natural key serialization to Add a default implementation of Model.natural_key()

Last status on this is that Russ was -0 on the mailing list, but hasn't responded to Jian's follow-up.

comment:3 Changed 15 months ago by russellm

Just commented on the mailing list thread - I'm still -0.

comment:4 Changed 14 months ago by timo

  • Resolution set to wontfix
  • Status changed from new to closed

Reading the thread, I agree with Russ.

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