Code

Changes between Version 20 and Version 21 of DjangoSpecifications/Core/SingleInstance


Ignore:
Timestamp:
03/25/08 07:35:07 (6 years ago)
Author:
ramiro
Comment:

just formatting changes

Legend:

Unmodified
Added
Removed
Modified
  • DjangoSpecifications/Core/SingleInstance

    v20 v21  
    99 
    1010=== Inconsistency === 
    11 #5514 describes what I think is the main issue with the current behavior of the ORM. Having multiple instances can result in silent data losses. Even worse the actual result would depend on whether one used select_related or not: 
     11#5514 describes what I think is the main issue with the current behavior of the ORM. Having multiple instances can result in silent data losses. Even worse the actual result would depend on whether one used {{{select_related}}} or not: 
    1212{{{ 
     13#!python 
    1314class Operand(models.Model): 
    1415    value = models.IntField() 
     
    3031Models with self FKs will exhibit similar issues: 
    3132{{{ 
    32  
     33#!python 
    3334class Directory(models.Model): 
    3435   name = models.CharField() 
     
    4647The original goal of #17 was reducing memory use and query count by reusing the same instance instead of having a new one. Let's see a typical example of that: 
    4748{{{ 
     49#!python 
    4850class ArticleType(models.Model): 
    4951    name = models.CharField(maxlength=200) 
     
    6567}}} 
    6668 
    67 In the above example, type_of_article should only be queried for once, because it's already in memory. This is a '''very''' common use case for many developers. 
     69In the above example, {{{type_of_article}}} should only be queried for once, because it's already in memory. This is a '''very''' common use case for many developers. 
    6870 
    6971If you have a great number of articles and a smaller number of articletypes the performance/memory hit is staggering because: 
     
    8486 
    8587=== API === 
    86 The public API for this feature will be very simple. It will be a single optional parameter in the Meta class: 
     88The public API for this feature will be very simple. It will be a single optional parameter in the {{{Meta}}} class: 
    8789 
    8890{{{ 
     91#!python 
    8992class ArticleType(models.Model): 
    9093    name = models.CharField(maxlength=200) 
     
    98101== Implementation == 
    99102#17 currently has a working patch with the following: 
    100  * Per-model WeakValueDictionary with dict[pk_val] = instance. 
    101  * !__call!__ classmethod which either gets a instance from the dict or return a newly created one. 
     103 * Per-model {{{WeakValueDictionary}}} with {{{dict[pk_val] = instance}}}. 
     104 * {{{__call__}}} classmethod which either gets a instance from the dict or return a newly created one. 
    102105 * a simple interface for enabling or disabling the feature for a given Model (default is disabled). 
    103106 * a slightly modified related manager which will first try to get the instance from the Model dict instead of blindly querying the DB. 
     
    109112 * In order to properly handle shared instances, partial models, or modified models (such as those with extra select clauses) need to be wrapped in a class which will handle those modifications. 
    110113 * there is no doc as to how to enable/disable this feature and what to expect from it. 
    111  * the API for enabling/disabling the feature is very crude and consists only of two class methods. It should at the very least be possible to set the value as a member of the Meta class.  
     114 * the API for enabling/disabling the feature is very crude and consists only of two class methods. It should at the very least be possible to set the value as a member of the {{{Meta}}} class.  
    112115 * the Model dict should be set threadlocally. 
    113116 * it should be possible to force instantiation, because the serializers want that. 
    114  * For the sake of completeness, it should be possible to do Model.objects.get_from_db(id=something) to bypass the singleton. 
     117 * For the sake of completeness, it should be possible to do {{{Model.objects.get_from_db(id=something)}}} to bypass the singleton. 
    115118 
    116119== Discussions == 
    117120=== Internal API === 
    118 ''(David)'' The current patch design overrides the !__call!__ method for the model base which means it automatically returns singleton instances when available. This is convenient because XXX (David or Brian: feel free to fill this out). 
     121''(David)'' The current patch design overrides the {{{__call__}}} method for the model base which means it automatically returns singleton instances when available. This is convenient because XXX (David or Brian: feel free to fill this out). 
    119122 
    120123''(Philippe)'' Another approach would be to make this explicit and have a simple API along the lines of: 
    121124{{{ 
     125#!python 
    122126class ModelBase: 
    123127    def get_singleton(pk, kwargs = None): 
     
    125129     
    126130}}} 
    127 That means writing Model(kwargs) would return a freshly instantiated object (as is the case now) and bypass the singleton instance. Since Django's ORM would use the API internally, doing Model.objects.get would (by default, overridable in the manager) return the singleton. The default related manager would also have a new get_from_db method to force retrieving and instantiating from the DB only.  
     131That means writing {{{Model(kwargs)}}} would return a freshly instantiated object (as is the case now) and bypass the singleton instance. Since Django's ORM would use the API internally, doing {{{Model.objects.get}}} would (by default, overridable in the manager) return the singleton. The default related manager would also have a new {{{get_from_db}}} method to force retrieving and instantiating from the DB only.  
    128132 
    129133=== Default behavior === 
     
    131135 * ''(Philippe)'' It should be because it really makes the ORM behave more correctly but for the sake of backward compatibility let's fist commit off by default. We can change the default parameter value later, and in the meantime honor a global setting to turn it on for all models. 
    132136 
    133  * ''(David)'' This would always be enabled. You cannot turn it off. If stuff is broken by having this on then it should be broken. This can cause problems with serializers, and things that modify querysets and shouldn't (e.g. .extra). That is the only reason it should be turned off. If those problems are addressed there is no reason to use #17 and to have it enabled 100% of the time. 
     137 * ''(David)'' This would always be enabled. You cannot turn it off. If stuff is broken by having this on then it should be broken. This can cause problems with serializers, and things that modify querysets and shouldn't (e.g. {{{.extra}}}). That is the only reason it should be turned off. If those problems are addressed there is no reason to use #17 and to have it enabled 100% of the time. 
    134138 
    135139=== extra, values === 
    136140 
    137 A simple workaround for querysets issues, and this would branch into being able to store .values() as partials as well (this code is untested, but should give a general idea): 
     141A simple workaround for querysets issues, and this would branch into being able to store {{{.values()}}} as partials as well (this code is untested, but should give a general idea): 
    138142 
    139143{{{ 
     144#!python 
    140145class SphinxObjectInstance(object): 
    141146    """ 
     
    160165}}} 
    161166 
    162 The proposal would also change how extra is used, it should probably be instance._extra or something similar. 
     167The proposal would also change how extra is used, it should probably be {{{instance._extra}}} or something similar. 
    163168 
    164169(The code above is taken from an unpublished update to the django-sphinx package.)