[Pulp-dev] ContentViewset assumes that POST is needed for any content

[David Davis]

On Wed, Oct 2, 2019 at 12:37 PM Brian Bouterse <bmbouter at redhat.com> wrote:

>
>
> On Wed, Oct 2, 2019 at 11:03 AM Tatiana Tereshchenko <ttereshc at 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/.


>
> 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 <dellweg at 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 <ttereshc at 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 at redhat.com
>> https://www.redhat.com/mailman/listinfo/pulp-dev
>>
> _______________________________________________
> Pulp-dev mailing list
> Pulp-dev at redhat.com
> https://www.redhat.com/mailman/listinfo/pulp-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/pulp-dev/attachments/20191002/1084c816/attachment.htm>