I like your solution using default_related_name [0] manually, as Brian noted [1], it's more explicit and therefore more pythonic.
That in mind, Daniel's alternative, not using model inheritance for the Content models [2], while less simple a change initially, potentially had significant performance gains and is also more explicit and pythonic. Should we still pursue this more complex fix for the improvements to bulk_create since we'd rather have breaking changes early in development than need to address them later? Or am I putting the cart before the horse by seeking optimization too early? [0] https://pulp.plan.io/issues/4681#note-19 [1] https://pulp.plan.io/issues/4681#note-20 [2] https://pulp.plan.io/issues/4681#note-11 Dana Walker She / Her / Hers Software Engineer, Pulp Project Red Hat <https://www.redhat.com> dawal...@redhat.com <https://www.redhat.com> On Wed, Jul 24, 2019 at 8:24 AM David Davis <davidda...@redhat.com> wrote: > 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 >
_______________________________________________ Pulp-dev mailing list Pulp-dev@redhat.com https://www.redhat.com/mailman/listinfo/pulp-dev
