[Pulp-dev] serializers in pulp 3 can't use write_only fields

[Dennis Kliban]

This discussion grew out of concern for the users of bindings. These users
don't usually instantiate models when performing GET requests. Instead,
they pass in a pulp_href to an api client object and receive an
instantiated model. On the other hand, when performing POST/PATCH
operations, the user does instantiate a model. So another solution to the
problem is to have the openapi generator automatically split the
serializers into two when write_only fields are present. However, only
rename the serializer used for GET operations. In this case, users will be
guaranteed that models that they do instantiate themselves, will never
change in name. If there is code that performs some kind of type checking
on the model returned by a GET operation, the user would need to update
that code.

I prefer this solution.

On Mon, Apr 27, 2020 at 9:11 AM Daniel Alley <dalley at redhat.com> wrote:

> I just want to point out that we have a somewhat similar kind of feature
> implemented, "minimal serializers", which uses the 2-serializer approach.
> It is a very tiny amount of extra code/work.
>
> e.g.
>
> https://github.com/pulp/pulpcore/blob/master/pulpcore/app/serializers/task.py#L119
>
> https://github.com/pulp/pulp_rpm/blob/master/pulp_rpm/app/serializers.py#L290
>
> the implementation
>
> https://github.com/pulp/pulpcore/blob/master/pulpcore/app/viewsets/base.py#L114-L139
>
> I lean towards option 2 as well for consistency.
>
> On Mon, Apr 27, 2020 at 5:20 AM Ina Panova <ipanova at redhat.com> wrote:
>
>> I like option number 2 more, even if there is more work behind the scene
>> it is more explicit and does not leave opened questions like ' why certain
>> fields are null'
>> --------
>> Regards,
>>
>> Ina Panova
>> Senior 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 Fri, Apr 24, 2020 at 5:24 PM Dennis Kliban <dkliban at redhat.com> wrote:
>>
>>> Today during open floor we concluded that write_only fields cannot be
>>> used in serializers because any automated handling of them during OpenAPI
>>> schema generation will cause mutations to the schema which will result in
>>> backwards incompatible changes to our bindings users.
>>>
>>> This change should only affect the Content serializers that have a
>>> 'file' and 'repository' write_only fields that can be used for uploading
>>> content into a repository in a single request. Two solutions were proposed:
>>>
>>> 1) Use a single serializer for GET and POST and leave 'file' and
>>> 'repository' null for GET requests. This solution is the simplest for
>>> plugin writers because they don't have to do anything extra. However, it
>>> does cause users to wonder why those fields are present and null on GET
>>> requests.
>>>
>>> 2) Require plugin writers to provide 2 serializers for Content - one for
>>> GET and another for POST. This is more work for plugin writers, but will
>>> make it clear to users what fields are expected in each case.
>>>
>>> Even though I initially proposed 1, I am in favor of adopting 2. What
>>> does everyone else think?
>>>
>>> We will need to provide validation at startup that will check
>>> serializers for presence of write_only fields. The application should fail
>>> to start if any are present. This way plugin writers will know not to use
>>> the fields. We will also need to provide plugin writer docs that clearly
>>> state the guidelines around write_only fields and suggestions for getting
>>> by without them.
>>> _______________________________________________
>>> 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/20200429/f100864f/attachment.htm>