[Pulp-dev] Master-detail inheritance in Pulp 3

[Brian Bouterse]

Thanks for the investigation and demo patch. I posted a +1 for the manual
option with reasoning here: https://pulp.plan.io/issues/4681#note-20

Other ideas and perspectives are welcome. I hope we can resolve this issue
soon as we approach RC4.

On Wed, Jul 17, 2019 at 12:55 PM David Davis <daviddavis at redhat.com> wrote:

> I did some investigation and posted my findings[0]. Basically, it would be
> possible to solve this problem by defining default_related_name either
> manually or automatically on detail models. I don't know if we want to go
> this route so feedback is appreciated.
>
> [0] https://pulp.plan.io/issues/4681#note-19
>
> David
>
>
> On Mon, Apr 29, 2019 at 2:16 PM David Davis <daviddavis at redhat.com> wrote:
>
>> It seems like most people are in favor of setting the OneToOneField or
>> perhaps the default_related_name on the detail model. I think there’s also
>> some interest in seeing how we can do this automatically for plugins. I’ve
>> added this feedback to the issue:
>>
>> https://pulp.plan.io/issues/4681#note-8
>>
>> David
>>
>>
>> On Wed, Apr 24, 2019 at 6:22 AM Ina Panova <ipanova at redhat.com> wrote:
>>
>>> I would avoid making changes in class naming. So +1 for the
>>> OneToOneField definition.
>>>
>>>
>>> --------
>>> Regards,
>>>
>>> Ina Panova
>>> Software Engineer| Pulp| Red Hat Inc.
>>>
>>> "Do not go where the path may lead,
>>>  go instead where there is no path and leave a trail."
>>>
>>>
>>> On Tue, Apr 23, 2019 at 6:45 PM David Davis <daviddavis at redhat.com>
>>> wrote:
>>>
>>>> The default_related_name setting is something that django provides.
>>>> Subclasses can also explicitly define their OneToOneField parent link as
>>>> well:
>>>>
>>>> content_ptr = models.OneToOneField(Content, on_delete=models.CASCADE,
>>>> parent_link=True, related_name='rpm_package')
>>>>
>>>> I am not sure what you mean by 'robust' but if a plugin subclass
>>>> doesn't do either of these, it may not work with other plugins.
>>>>
>>>> I think the question now would be whether we should just document this
>>>> or try to do it automagically for plugins?
>>>>
>>>> David
>>>>
>>>>
>>>> On Tue, Apr 23, 2019 at 12:31 PM Brian Bouterse <bbouters at redhat.com>
>>>> wrote:
>>>>
>>>>>
>>>>>
>>>>> On Tue, Apr 23, 2019 at 11:02 AM David Davis <daviddavis at redhat.com>
>>>>> wrote:
>>>>>
>>>>>> I think I found another solution that might work best: defining
>>>>>> 'default_related_name' on subclassed master-detail models. So Package in
>>>>>> pulp_rpm would define its default_related_name as "rpm_package".
>>>>>>
>>>>> Would we be making 'default_related_name' or is that something Django
>>>>> is providing? If it's something Pulp would be providing perhaps defining
>>>>> the explicit one-to-one field is better. Any plugin that takes the step of
>>>>> defining the one-to-one field will insulate themselves from other plugins.
>>>>> If plugins don't take that step they will still work, just not as robustly.
>>>>> Am I thinking about this correctly?
>>>>>
>>>>>
>>>>>> David
>>>>>>
>>>>>>
>>>>>> On Tue, Apr 23, 2019 at 10:29 AM David Davis <daviddavis at redhat.com>
>>>>>> wrote:
>>>>>>
>>>>>>> I wanted to email the pulp-dev list about a major problem[0] that
>>>>>>> was recently encountered in Pulp 3 that affects how the Pulp 3 plugin API
>>>>>>> functions.
>>>>>>>
>>>>>>> # Problem
>>>>>>>
>>>>>>> In the plugin API we rely on inheritance to allow plugin writers to
>>>>>>> import functionality into their plugin. This includes models such as Remote
>>>>>>> and Content that are inherited by plugins. We rely on django's multi-table
>>>>>>> inheritance[1] for these models.
>>>>>>>
>>>>>>> Behind the scenes, django defines a OneToOneField and a reverse
>>>>>>> accessor. This field is not namespace so if two subclasses have the same
>>>>>>> name, you get an error ("Reverse accessor for 'Package.content_ptr' clashes
>>>>>>> with reverse accessor for 'Package.content_ptr'.")
>>>>>>>
>>>>>>> To give an actual example, both the Debian and RPM plugins implement
>>>>>>> a Package class. This causes an error to be raised when a user installs
>>>>>>> both plugins. Django tries to define a 'package' reverse accessor for both
>>>>>>> subclasses and blows up.
>>>>>>>
>>>>>>> # Potential Solutions
>>>>>>>
>>>>>>> ## Class Naming
>>>>>>>
>>>>>>> The first solution I can think of which is probably also the
>>>>>>> simplest and most straightforward would be to require plugin writers to
>>>>>>> namespace their master/detail subclass names. So Package would be
>>>>>>> RpmPackage. This places the onus on plugin writers to name their
>>>>>>> master/detail classes correctly.
>>>>>>>
>>>>>>> ## Defining OneToOneField
>>>>>>>
>>>>>>> The other solution would be to either manually define the
>>>>>>> OneToOneField on the subclasses[2] and specify a namespaced field name.
>>>>>>> There may be a way to do this dynamically (ie magically) in the parent
>>>>>>> somehow as well.
>>>>>>>
>>>>>>> ## Abstract Class
>>>>>>>
>>>>>>> Lastly, we could redefine master models as abstract classes[3]. I
>>>>>>> can think of at least one or two places (e.g. content field on
>>>>>>> RepositoryVersionContent, publisher field on Publication) that would have
>>>>>>> to switch their relationships to generic relationships in order to
>>>>>>> accommodate this change.
>>>>>>>
>>>>>>> There might be other solutions I am not thinking of so feel free to
>>>>>>> propose something. Also, quick feedback would be greatly appreciated as
>>>>>>> this is going to be a major change in our plugin API.
>>>>>>>
>>>>>>> [0] https://pulp.plan.io/issues/4681
>>>>>>> [1]
>>>>>>> https://docs.djangoproject.com/en/2.2/topics/db/models/#multi-table-inheritance
>>>>>>> [2]
>>>>>>> https://docs.djangoproject.com/en/2.2/topics/db/models/#specifying-the-parent-link-field
>>>>>>> [3]
>>>>>>> https://docs.djangoproject.com/en/2.2/topics/db/models/#abstract-base-classes
>>>>>>>
>>>>>>> David
>>>>>>>
>>>>>> _______________________________________________
>>>>>> Pulp-dev mailing list
>>>>>> Pulp-dev at redhat.com
>>>>>> https://www.redhat.com/mailman/listinfo/pulp-dev
>>>>>>
>>>>> _______________________________________________
>>>> Pulp-dev mailing list
>>>> Pulp-dev at redhat.com
>>>> https://www.redhat.com/mailman/listinfo/pulp-dev
>>>>
>>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/pulp-dev/attachments/20190721/ee274ac8/attachment.htm>