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

[Brian Bouterse]

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?

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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/pulp-dev/attachments/20191002/655391ce/attachment.htm>