GenericForeignKey: loosening the requirements on the ContentType reference
170 views
Skip to first unread message
martin f krafft
Dec 8, 2015, 8:10:51 AM12/8/15
to django-d...@googlegroups.com
Hello,
I hope this is the right place to bring up the following idea, even
if it's in django.contrib.contenttypes.
GenericForeignKeys currently hard-require a model to have
a ForeignKey field to the ContentType model. In a specific use-case,
we're experiencing a bit of trouble with this, because the
ContentType in our case is actually indirectly available via another
ForeignKey:
MyModel → n:1 → ParentModel → n:1 → ContentType
However, contenttypes cannot be convinced to access the extra level
of indirection and complains that MyModel.ForeignKey('ParentModel')
is not a ForeignKey('ContentType').
Despite the additional query overhead, it'd be great if instead
I could tell contenttypes that it should
either
(a) look beyond the current model for the actual content_type
attribute, e.g. content_type='myattribute.content_type',
which, if myattribute is a ForeignKey itself adds a level of
indirection but doesn't change the logic and causes
contenttypes to resolve myattribute and use the content_type
field on the result,
or
(b) avoid hard-coding the dependency on ContentType and instead
duck-type the argument, such that my parent model could expose
the necessary functionality, e.g. via proxy calls to the
ForeignKey on ContentType stored in the parent.
Does this make sense? Would it be worthwhile to start working on
a patch for this? Which of (a) or (b) would be preferred? It feels
to me that while (b) is the Pythonesque way, (a) is more Django…
Looking forward to your thoughts,
--
@martinkrafft | http://madduck.net/ | http://two.sentenc.es/
warning: dates in calendar are closer than they appear.
spamtraps: madduc...@madduck.net
I hope this is the right place to bring up the following idea, even
if it's in django.contrib.contenttypes.
GenericForeignKeys currently hard-require a model to have
a ForeignKey field to the ContentType model. In a specific use-case,
we're experiencing a bit of trouble with this, because the
ContentType in our case is actually indirectly available via another
ForeignKey:
MyModel → n:1 → ParentModel → n:1 → ContentType
However, contenttypes cannot be convinced to access the extra level
of indirection and complains that MyModel.ForeignKey('ParentModel')
is not a ForeignKey('ContentType').
Despite the additional query overhead, it'd be great if instead
I could tell contenttypes that it should
either
(a) look beyond the current model for the actual content_type
attribute, e.g. content_type='myattribute.content_type',
which, if myattribute is a ForeignKey itself adds a level of
indirection but doesn't change the logic and causes
contenttypes to resolve myattribute and use the content_type
field on the result,
or
(b) avoid hard-coding the dependency on ContentType and instead
duck-type the argument, such that my parent model could expose
the necessary functionality, e.g. via proxy calls to the
ForeignKey on ContentType stored in the parent.
Does this make sense? Would it be worthwhile to start working on
a patch for this? Which of (a) or (b) would be preferred? It feels
to me that while (b) is the Pythonesque way, (a) is more Django…
Looking forward to your thoughts,
--
@martinkrafft | http://madduck.net/ | http://two.sentenc.es/
warning: dates in calendar are closer than they appear.
spamtraps: madduc...@madduck.net
Tim Graham
Dec 8, 2015, 4:38:27 PM12/8/15
to Django developers (Contributions to Django itself), mad...@madduck.net
I'm having trouble understanding the problem. Could you give some example models and code that demonstrates it?
Florian Apolloner
Dec 8, 2015, 5:38:24 PM12/8/15
to Django developers (Contributions to Django itself), mad...@madduck.net
In addition to what Tim already said, I think the acceptance of a patch on such a feature would depend on the (quality of the) patch itself. I am not going to spelk for the whole core team here, but I think most of the people in the core team try to avoid generic FKs for plenty of reasons. As such a change in this area might have better chances with an existing patch showing what you are trying to achieve. I know, that this puts a lot of pressure on you, or at least asks for work which might soon be forgotten -- but if it were for me GFK would have a special case in hell.
Cheers,
Florian
Cheers,
Florian
Shai Berger
Dec 8, 2015, 7:20:30 PM12/8/15
to django-d...@googlegroups.com
On Tuesday 08 December 2015 23:38:26 Tim Graham wrote:
> I'm having trouble understanding the problem. Could you give some example
> models and code that demonstrates it?
>
If I get things right:
> I'm having trouble understanding the problem. Could you give some example
> models and code that demonstrates it?
>
class Parent(Model):
content_type = ForeignKey('ContentType')
class Child(Model):
parent = ForeignKey(Parent)
object_id = IntegerField()
# The next does not exist and they want it
egfk = ExtendedGenericForeignKey('parent__content_type', 'object_id')
I suspect that this is a little too baroque for django core, and further, it
should be possible to implement in user code. Note that GenericForeignKey
itself is not part of Django core, but a contrib app; there should be no
reason why you cannot do what it does (with the only possible exception being
admin support).
If you try to do that and find that you are missing something, or need to use
undocumented, private APIs, you'll have a very strong case for having those
problems solved.
Of course, if this is not what you had in mind, please correct my
misunderstanding.
HTH,
Shai.
martin f. krafft
Dec 8, 2015, 10:59:20 PM12/8/15
to Tim Graham, Django developers (Contributions to Django itself)
also sprach Tim Graham <timog...@gmail.com> [2015-12-08 22:38 +0100]:
https://github.com/madduck/wafer/blob/147-kvpairs/wafer/kvpairs/models.py
https://github.com/madduck/wafer/blob/147-kvpairs/docs/kvpairs.rst
but I'll try to give the quick rundown here too:
Basic requirement: avoid creating new database fields for all the
various random needs around conference management. In the past, we'd
add to the UserProfile fields to hold t-shirt size, payment and
arrival status, day trip participation etc. and the database just
grew and grew. We're now switching to this new "wafer" and would
like to ensure that this doesn't happen again.
So while there are some profile attributes for sure that are core
and will go into the profile table, others aren't and we'd really
rather treat them as throw-away, quick'n'dirty data, which they
often are.
Hence I created this key-value store. At the core, it just attaches
two text fields "key" and "value" to an instance of a model (e.g.
a scheduled talk), and GenericForeignKey would be just perfect for
this.
But we also wanted to collect a bit of information about a Key and
add a bit more structure. For the moment, Key instances just hold
information about owner and group for access control and
namespacing, and, well, the model to which this key applies (also
used for namespacing).
Here's what it looks like:
+-- ContentType --+ +-- Key ----+ 1 +-- KVPair --+
| ID |-. | ID |---. n | ID |
| app_label | \1 | owner | `--| object_id |
| model | \ | group | .--| ref_id |
| name | n\ | name | n| | value |
+-----------------+ `-| content_t | | +------------+
+-----------+ |
1| +-- Talk --+
`--| ID |
| title |
+ … -------+
The problem is that GenericForeignKey expects content_type and
object_id to be in the KVPair model, but content_type is in the Key.
There's still a 1:n relation between ContentType and KVPair, but
it's indirect.
You might rightfully ask why we don't just throw the Key fields into
KVPair and be done with it. Sure, that is a solution, but you'll
also concur that it'll involve a lot of duplicate data and make
unique constraints a lot harder. It'd seem a workaround at best for
the limitations around GenericForeignKey.
But if GenericForeignKey could just be appeased by telling it "yes
I know you want content_type in the same model, but trust me, if you
use the attribute of this related object, you'll get the same", then
it'd work out.
To me, the even better solution would be to give Key the right
accessors to make GenericForeignKey happy, such that GFK doesn't
even need to care about what kind of object it's being asked to
refer to, as long as it gets the data it needs. I have toyed with
the idea of making Key a subclass of ContentType, but that creates
a slew of other problems, and doesn't address the fact tat
GenericForeignKey actually compares the instance class with the
hard-coded string "contenttypes.GenericForeignKey".
Does this make more sense?
an egg has the shortest sex-life of all: if gets laid once; it gets
eaten once. it also has to come in a box with 11 others, and the
only person who will sit on its face is its mother.
spamtraps: madduc...@madduck.net
> I'm having trouble understanding the problem. Could you give some example
> models and code that demonstrates it?
Gladly. The code I am talking about is here:
> models and code that demonstrates it?
https://github.com/madduck/wafer/blob/147-kvpairs/wafer/kvpairs/models.py
https://github.com/madduck/wafer/blob/147-kvpairs/docs/kvpairs.rst
but I'll try to give the quick rundown here too:
Basic requirement: avoid creating new database fields for all the
various random needs around conference management. In the past, we'd
add to the UserProfile fields to hold t-shirt size, payment and
arrival status, day trip participation etc. and the database just
grew and grew. We're now switching to this new "wafer" and would
like to ensure that this doesn't happen again.
So while there are some profile attributes for sure that are core
and will go into the profile table, others aren't and we'd really
rather treat them as throw-away, quick'n'dirty data, which they
often are.
Hence I created this key-value store. At the core, it just attaches
two text fields "key" and "value" to an instance of a model (e.g.
a scheduled talk), and GenericForeignKey would be just perfect for
this.
But we also wanted to collect a bit of information about a Key and
add a bit more structure. For the moment, Key instances just hold
information about owner and group for access control and
namespacing, and, well, the model to which this key applies (also
used for namespacing).
Here's what it looks like:
+-- ContentType --+ +-- Key ----+ 1 +-- KVPair --+
| ID |-. | ID |---. n | ID |
| app_label | \1 | owner | `--| object_id |
| model | \ | group | .--| ref_id |
| name | n\ | name | n| | value |
+-----------------+ `-| content_t | | +------------+
+-----------+ |
1| +-- Talk --+
`--| ID |
| title |
+ … -------+
The problem is that GenericForeignKey expects content_type and
object_id to be in the KVPair model, but content_type is in the Key.
There's still a 1:n relation between ContentType and KVPair, but
it's indirect.
You might rightfully ask why we don't just throw the Key fields into
KVPair and be done with it. Sure, that is a solution, but you'll
also concur that it'll involve a lot of duplicate data and make
unique constraints a lot harder. It'd seem a workaround at best for
the limitations around GenericForeignKey.
But if GenericForeignKey could just be appeased by telling it "yes
I know you want content_type in the same model, but trust me, if you
use the attribute of this related object, you'll get the same", then
it'd work out.
To me, the even better solution would be to give Key the right
accessors to make GenericForeignKey happy, such that GFK doesn't
even need to care about what kind of object it's being asked to
refer to, as long as it gets the data it needs. I have toyed with
the idea of making Key a subclass of ContentType, but that creates
a slew of other problems, and doesn't address the fact tat
GenericForeignKey actually compares the instance class with the
hard-coded string "contenttypes.GenericForeignKey".
Does this make more sense?
an egg has the shortest sex-life of all: if gets laid once; it gets
eaten once. it also has to come in a box with 11 others, and the
only person who will sit on its face is its mother.
spamtraps: madduc...@madduck.net
martin f. krafft
Dec 8, 2015, 11:01:32 PM12/8/15
to Django developers (Contributions to Django itself)
also sprach Florian Apolloner <f.apo...@gmail.com> [2015-12-08 23:38 +0100]:
"as if you could kill time without injuring eternity."
-- henry david thoreau
spamtraps: madduc...@madduck.net
> but if it were for me GFK would have a special case in hell.
How would you implement LogEntries without GFKs?
"as if you could kill time without injuring eternity."
-- henry david thoreau
spamtraps: madduc...@madduck.net
martin f krafft
Dec 8, 2015, 11:07:50 PM12/8/15
to django-d...@googlegroups.com
also sprach Shai Berger <sh...@platonix.com> [2015-12-09 01:20 +0100]:
> I suspect that this is a little too baroque for django core, and
> further, it should be possible to implement in user code. Note
> that GenericForeignKey itself is not part of Django core, but
> a contrib app;
Yes, and as noted in my initial mail: I wasn't even sure if turning
to this list was the right thing to do.
> there should be no reason why you cannot do what it does (with the
> only possible exception being admin support).
Yes, I have it all implemented, but it just seems like I am hacking
and duplicating code and it feels aweful. Thus my itch to make the
upstream code more flexible.
> If you try to do that and find that you are missing something, or
> need to use undocumented, private APIs, you'll have a very strong
> case for having those problems solved.
Isn't this enough:
https://github.com/django/django/blob/stable/1.9.x/django/contrib/contenttypes/fields.py#L151
It hardcodes the class object and thereby prevents any
subclassing/specialisation/extension, and it's not at all how Python
was designed to work. I understand that the checks framework
requires one to step beyond simple Python duck-typing, but I am not
sure I can be convinced that it's necessary to enforce a specific
model class, when all you need to make sure is that the related
object exposes a specific interface.
"this week dragged past me so slowly;
the days fell on their knees..."
-- david bowie
spamtraps: madduc...@madduck.net
> class Parent(Model):
> content_type = ForeignKey('ContentType')
>
> class Child(Model):
> parent = ForeignKey(Parent)
> object_id = IntegerField()
> # The next does not exist and they want it
> egfk = ExtendedGenericForeignKey('parent__content_type', 'object_id')
Precisely.
> content_type = ForeignKey('ContentType')
>
> class Child(Model):
> parent = ForeignKey(Parent)
> object_id = IntegerField()
> # The next does not exist and they want it
> egfk = ExtendedGenericForeignKey('parent__content_type', 'object_id')
> I suspect that this is a little too baroque for django core, and
> further, it should be possible to implement in user code. Note
> that GenericForeignKey itself is not part of Django core, but
> a contrib app;
to this list was the right thing to do.
> there should be no reason why you cannot do what it does (with the
> only possible exception being admin support).
and duplicating code and it feels aweful. Thus my itch to make the
upstream code more flexible.
> If you try to do that and find that you are missing something, or
> need to use undocumented, private APIs, you'll have a very strong
> case for having those problems solved.
https://github.com/django/django/blob/stable/1.9.x/django/contrib/contenttypes/fields.py#L151
It hardcodes the class object and thereby prevents any
subclassing/specialisation/extension, and it's not at all how Python
was designed to work. I understand that the checks framework
requires one to step beyond simple Python duck-typing, but I am not
sure I can be convinced that it's necessary to enforce a specific
model class, when all you need to make sure is that the related
object exposes a specific interface.
"this week dragged past me so slowly;
the days fell on their knees..."
-- david bowie
spamtraps: madduc...@madduck.net
Shai Berger
Dec 9, 2015, 2:39:05 PM12/9/15
to django-d...@googlegroups.com
On Wednesday 09 December 2015 06:07:38 martin f krafft wrote:
> also sprach Shai Berger <sh...@platonix.com> [2015-12-09 01:20 +0100]:
> > class Parent(Model):
> > content_type = ForeignKey('ContentType')
> >
> > class Child(Model):
> > parent = ForeignKey(Parent)
> > object_id = IntegerField()
> > # The next does not exist and they want it
> > egfk = ExtendedGenericForeignKey('parent__content_type', 'object_id')
>
> Precisely.
>
> > I suspect that this is a little too baroque for django core, and
> > further, it should be possible to implement in user code. Note
> > that GenericForeignKey itself is not part of Django core, but
> > a contrib app;
>
> Yes, and as noted in my initial mail: I wasn't even sure if turning
> to this list was the right thing to do.
>
> > there should be no reason why you cannot do what it does (with the
> > only possible exception being admin support).
>
> Yes, I have it all implemented, but it just seems like I am hacking
> and duplicating code and it feels aweful. Thus my itch to make the
> upstream code more flexible.
>
As far as I can see, you are trying to implement it in the model, rather than
> also sprach Shai Berger <sh...@platonix.com> [2015-12-09 01:20 +0100]:
> > class Parent(Model):
> > content_type = ForeignKey('ContentType')
> >
> > class Child(Model):
> > parent = ForeignKey(Parent)
> > object_id = IntegerField()
> > # The next does not exist and they want it
> > egfk = ExtendedGenericForeignKey('parent__content_type', 'object_id')
>
> Precisely.
>
> > I suspect that this is a little too baroque for django core, and
> > further, it should be possible to implement in user code. Note
> > that GenericForeignKey itself is not part of Django core, but
> > a contrib app;
>
> Yes, and as noted in my initial mail: I wasn't even sure if turning
> to this list was the right thing to do.
>
> > there should be no reason why you cannot do what it does (with the
> > only possible exception being admin support).
>
> Yes, I have it all implemented, but it just seems like I am hacking
> and duplicating code and it feels aweful. Thus my itch to make the
> upstream code more flexible.
>
as a new kind of field. Try to write ExtendedGenericForeignKey, and I think
things would look much better...
> > If you try to do that and find that you are missing something, or
> > need to use undocumented, private APIs, you'll have a very strong
> > case for having those problems solved.
>
> Isn't this enough:
>
>
> https://github.com/django/django/blob/stable/1.9.x/django/contrib/contentt
> ypes/fields.py#L151
>
HTH,
Shai.
Florian Apolloner
Dec 10, 2015, 5:45:15 AM12/10/15
to Django developers (Contributions to Django itself), mad...@madduck.net
On Wednesday, December 9, 2015 at 5:01:32 AM UTC+1, martin f krafft wrote:
also sprach Florian Apolloner <f.apo...@gmail.com> [2015-12-08 23:38 +0100]:
> but if it were for me GFK would have a special case in hell.
How would you implement LogEntries without GFKs?
Just put the table name in as string…
martin f krafft
Dec 30, 2015, 1:40:31 AM12/30/15
to django-d...@googlegroups.com
also sprach Shai Berger <sh...@platonix.com> [2015-12-10 08:38 +1300]:
to follow-up.
I've tried implementing this as a field at the very beginning, but
I had an incredibly hard time figuring out which methods I need to
provide. I don't suppose you know of any guidelines, other than the
GenericForeignKey code (which I've still not fully understood) that
could help?
"you don't sew with a fork, so I see no reason
to eat with knitting needles."
-- miss piggy, on eating chinese food
spamtraps: madduc...@madduck.net
> As far as I can see, you are trying to implement it in the model,
> rather than as a new kind of field. Try to write
> ExtendedGenericForeignKey, and I think things would look much
> better...
Hi, thanks for your response and sorry for taking such a long time
> rather than as a new kind of field. Try to write
> ExtendedGenericForeignKey, and I think things would look much
> better...
to follow-up.
I've tried implementing this as a field at the very beginning, but
I had an incredibly hard time figuring out which methods I need to
provide. I don't suppose you know of any guidelines, other than the
GenericForeignKey code (which I've still not fully understood) that
could help?
"you don't sew with a fork, so I see no reason
to eat with knitting needles."
-- miss piggy, on eating chinese food
spamtraps: madduc...@madduck.net
Shai Berger
Dec 31, 2015, 4:54:17 PM12/31/15
to django-d...@googlegroups.com
On Wednesday 30 December 2015 08:40:17 martin f krafft wrote:
> also sprach Shai Berger <sh...@platonix.com> [2015-12-10 08:38 +1300]:
> > As far as I can see, you are trying to implement it in the model,
> > rather than as a new kind of field. Try to write
> > ExtendedGenericForeignKey, and I think things would look much
> > better...
>
> Hi, thanks for your response and sorry for taking such a long time
> to follow-up.
>
No need to apologize, we all have lives outside of Django...
> also sprach Shai Berger <sh...@platonix.com> [2015-12-10 08:38 +1300]:
> > As far as I can see, you are trying to implement it in the model,
> > rather than as a new kind of field. Try to write
> > ExtendedGenericForeignKey, and I think things would look much
> > better...
>
> Hi, thanks for your response and sorry for taking such a long time
> to follow-up.
>
> I've tried implementing this as a field at the very beginning, but
> I had an incredibly hard time figuring out which methods I need to
> provide. I don't suppose you know of any guidelines, other than the
> GenericForeignKey code (which I've still not fully understood) that
> could help?
rather than a guideline.
Shai.
Reply all
Reply to author
Forward
0 new messages