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

Wed Jul 24 12:06:59 UTC 2019

I want to bump this thread again. We've only had one person weigh in and
this is a major change that'll affect all Pulp 3 plugins that we need to
address soon. Please respond here or on the issue with feedback.

David

On Sun, Jul 21, 2019 at 10:49 AM Brian Bouterse <bbouters at redhat.com> wrote:

> 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/20190724/ac6503bd/attachment.htm>