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
