Re: [Pulp-dev] serializers in pulp 3 can't use write_only fields
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 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 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 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@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
Re: [Pulp-dev] serializers in pulp 3 can't use write_only fields
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 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 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@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
Re: [Pulp-dev] serializers in pulp 3 can't use write_only fields
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 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@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] serializers in pulp 3 can't use write_only fields
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@redhat.com https://www.redhat.com/mailman/listinfo/pulp-dev
