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
