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 <bbout...@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 <davidda...@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 <davidda...@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 <ipan...@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 <davidda...@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 <bbout...@redhat.com> >>>>> wrote: >>>>> >>>>>> >>>>>> >>>>>> On Tue, Apr 23, 2019 at 11:02 AM David Davis <davidda...@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 <davidda...@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@redhat.com >>>>>>> https://www.redhat.com/mailman/listinfo/pulp-dev >>>>>>> >>>>>> _______________________________________________ >>>>> Pulp-dev mailing list >>>>> Pulp-dev@redhat.com >>>>> https://www.redhat.com/mailman/listinfo/pulp-dev >>>>> >>>>
_______________________________________________ Pulp-dev mailing list Pulp-dev@redhat.com https://www.redhat.com/mailman/listinfo/pulp-dev
