I filed an issue[0] and opened a couple PRs which demonstrate how to fix the problem:
https://github.com/pulp/pulpcore/pull/319 https://github.com/pulp/pulp_rpm/pull/1465 [0] https://pulp.plan.io/issues/5533 David On Wed, Oct 2, 2019 at 1:32 PM Tatiana Tereshchenko <ttere...@redhat.com> wrote: > +1 to what David said. Thanks for detailed explanation. > > +1 to file an issue to make it possible to define a custom endpoint for a > Detail model and keep automatic prepending at the same time. > > > On Wed, Oct 2, 2019 at 7:06 PM David Davis <davidda...@redhat.com> wrote: > >> No, it's more complicated than that. There's a bit of magic for >> Master/Detail model endpoints[0] where the NamedModelViewSet tries to form >> the endpoint by looking up the master_endpoint, app_label, and >> endpoint_name (and combining them). In the case of distribution_trees, >> there is no master viewset so the master_endpoint becomes >> 'distribution_trees' and thus the url we end up with is >> /pulp/api/v3/distribution_trees/rpm/distribution_trees/. If we used >> 'content', it would just be /pulp/api/v3/distribution_trees/rpm/content/. >> >> This is a problem we should probably fix regardless of how we handle >> Tanya's problem. To restate the problem: it's impossible (or very hard) to >> create your own endpoint for a Detail model because the code automatically >> tries to namespace your endpoint for you. I can file an issue if others >> agree. >> >> [0] >> https://github.com/pulp/pulpcore/blob/5bd0f31604de3079c8bb1b710155d7c6e94d7425/pulpcore/app/viewsets/base.py#L185-L216 >> >> David >> >> >> On Wed, Oct 2, 2019 at 12:52 PM Brian Bouterse <bmbou...@redhat.com> >> wrote: >> >>> >>> >>> On Wed, Oct 2, 2019 at 12:44 PM David Davis <davidda...@redhat.com> >>> wrote: >>> >>>> >>>> On Wed, Oct 2, 2019 at 12:37 PM Brian Bouterse <bmbou...@redhat.com> >>>> wrote: >>>> >>>>> >>>>> >>>>> On Wed, Oct 2, 2019 at 11:03 AM Tatiana Tereshchenko < >>>>> ttere...@redhat.com> wrote: >>>>> >>>>>> Thank you! Good idea to have an additional one. >>>>>> >>>>> I agree >>>>> >>>>> I'm on the fence between ReadOnly and just Generic one without any >>>>>> mixins. >>>>>> >>>>> I see having the ReadOnlyContentViewset as overall being slightly >>>>> better than a Generic option for plugin writers for two reasons. >>>>> >>>>> 1) the majority of cases (if not all) will want both list and get() >>>>> mixins so we're handling the common case with this. >>>>> 2) In the event a plugin writer needs to not have list and/or get(), >>>>> they could could use NamedModelViewset directly still. Is this accurate? >>>>> >>>> >>>> Per Tanya's original email it sounds like it was hard/impossible to >>>> roll a viewset at /pulp/api/v3/content/rpm/distribution_trees/. >>>> >>> >>> Oh I see, but isn't that an outcome of `endpoint_name = ' >>> distribution_trees'` here >>> <https://github.com/pulp/pulp_rpm/blob/master/pulp_rpm/app/viewsets.py#L256>? >>> What happens if that becomes `endpoint_name = 'content'`. I believe the >>> NamedModelViewset would translate that into the url >>> /pulp/api/v3/content/rpm/distribution_trees/ where the `distribution_trees` >>> part comes fro the classname. I haven't looking into it in a while though, >>> so let me know if I'm still not getting it right. >>> >>> In the event it's not easy for the plugin writer to be in control of >>> that then I would be in favor of GenericContentViewset. >>> >>> >>>> >>>>> >>>>> Just to write it out for clarity I imagine it would be: >>>>> ReadOnlyContentViewset(NamedModelViewset, mixins.RetrieveModelMixin, >>>>> mixins.ListModelMixin) >>>>> >>>>> What do you think? >>>>> >>>>> >>>>>> Any other opinions/suggestions? >>>>>> >>>>>> On Wed, Oct 2, 2019 at 4:35 PM Matthias Dellweg <dell...@atix.de> >>>>>> wrote: >>>>>> >>>>>>> I would do a variation of 1. : >>>>>>> Provide a ReadonlyContentViewSet with only GET mixed in and leave the >>>>>>> 'standard' ContentViewset as is. >>>>>>> Cheers, Matthias >>>>>>> >>>>>>> On Wed, 2 Oct 2019 16:16:14 +0200 >>>>>>> Tatiana Tereshchenko <ttere...@redhat.com> wrote: >>>>>>> >>>>>>> > Current implementation of ContentViewset >>>>>>> > < >>>>>>> https://github.com/pulp/pulpcore/blob/master/pulpcore/app/viewsets/content.py#L98-L102 >>>>>>> > >>>>>>> > includes >>>>>>> > mixins for create (POST) and retrieve/list (GET). >>>>>>> > In case a plugin doesn't need to support POST for a content >>>>>>> endpoint, >>>>>>> > a plugin writer compiles a viewset from the mixins they need, e.g. >>>>>>> > distribution trees and custom metadata >>>>>>> > < >>>>>>> https://github.com/pulp/pulp_rpm/blob/master/pulp_rpm/app/viewsets.py#L233-L258 >>>>>>> > >>>>>>> > (same >>>>>>> > use case is expected for modularity endpoints) >>>>>>> > This leads to the inconsistent REST API. >>>>>>> > >>>>>>> > # ContentViewset is used >>>>>>> > /pulp/api/v3/content/rpm/advisories/ >>>>>>> > >>>>>>> > # custom plugin content viewset >>>>>>> > /pulp/api/v3/distribution_trees/rpm/distribution_trees/ >>>>>>> > >>>>>>> > Possible solutions: >>>>>>> > 1. Make ContentViewset more generic (no mixins, or only GET ones?) >>>>>>> > and let plugins include any mixins they need. >>>>>>> > This option might be painful to switch to for plugin writers, >>>>>>> because >>>>>>> > every plugin will be affected and will need to make this change. >>>>>>> > At the same time probably not many plugins support upload for every >>>>>>> > content type, so in many cases the POST is broken/not used anyway. >>>>>>> > >>>>>>> > 2. Disable POST at the plugin level in some other way. >>>>>>> > I'm not sure if there is any native option to disable it. >>>>>>> > Hacky way is to override `create` method which will return >>>>>>> > appropriate HTTP error that POST is not supported. >>>>>>> > >>>>>>> > 3. Make plugin writers manually define a proper endpoint name. >>>>>>> > Apart from not being reliable, I'm not sure how to do it because of >>>>>>> > how we tweak endpoint generation. >>>>>>> > Notice the distribution trees example ^, "distribution_trees" is >>>>>>> used >>>>>>> > twice in the endpoint. >>>>>>> > >>>>>>> > 4. Any other solutions? Easy ones which I missed? >>>>>>> > >>>>>>> > Thank you, >>>>>>> > Tanya >>>>>>> >>>>>> _______________________________________________ >>>>>> 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
