<div dir="ltr"><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Feb 11, 2020 at 6:04 PM Lubos Mjachky <<a href="mailto:lmjachky@redhat.com">lmjachky@redhat.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">Dear colleagues,<div><br></div><div>functional tests are currently being refactored to use bindings in various plugins. This will eventually allow us to query Pulp in a more pythonic way instead of using raw REST API calls, or pulp_smash utilities.</div><div><br></div><div>While refactoring the functional tests in pulp_container, I noticed that sometimes it is necessary to send or receive data which surprisingly do not satisfy all the declared requirements in serializers because we want, for example, to test whether a feature was correctly implemented or not and whether errors are properly handled or not.</div><div><br></div><div>However, in the bindings, it is not always possible to accomplish that in a graceful way. To create a request with invalid data, one should not do this:</div><div><br></div><div><div><font face="monospace">distribution_data = ContainerContainerDistribution(**gen_distribution(foo="bar")) # raised an exception; got an unexpected keyword argument 'foo'</font></div><div><font face="monospace">with self.assertRaises(ApiException) as exc:<br> self.distribution_api.create(distribution)</font></div></div><div><br></div><div>But should do rather this:</div><div><div><br></div><div><font face="monospace">distribution_data = gen_distribution(foo="bar") # a simple dictionary</font></div><div><font face="monospace">with self.assertRaises(ApiException) as exc:<br> self.distributions_api.create(distribution_data)</font></div><div></div></div><div><br></div>To disable validation in data classes, one can pass a Configuration object with the attribute client_side_validation=False to the method __init__ of a corresponding data class, like so:<div><br></div><div><font face="monospace">configuration = Configuration()</font></div><div><span style="font-family:monospace">configuration.client_side_validation = False</span></div><div><span style="font-family:monospace">ContainerManifest(**manifest_without_required_field, </span><span style="font-family:monospace">local_vars_configuration=configuration)</span></div><div><font face="monospace"><br></font></div><div><font face="arial, sans-serif">Then we may use the first approach without any problems. This, in fact, disables the implicit validation for required fields which is turned on by default for every single data class.</font></div><div><font face="arial, sans-serif"><br></font></div><div><font face="arial, sans-serif">Another problem might be observed in generated methods for api calls (e.g. list, read, ...) which return data classes, such as ContainerManifest, where a Configuration object that was declared globally for ApiClient is ignored. This is based on my assumptions but I really could not find a way how to pass an existing configuration to these api calls. Due to that, a new Configuration is created separately in each data class and the validation is enabled again. For instance, the following call will fail, because in pulp_container, a manifest list does not have config_blob:</font></div><div><font face="arial, sans-serif"><br></font></div><div><font face="monospace">ml = manifests_api.read(ml_href) # raised an exception; invalid value for `config_blob`, must not be `None`</font><font face="arial, sans-serif"><br></font></div><div><font face="monospace"><br></font></div></div></blockquote><div><br></div><div>The problem here is that config_blob shouldn't be a required field. Let's fix that in the serializer. <br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div><font face="monospace"></font></div><div><font face="arial, sans-serif">Instead, you should do the following:</font></div><div><br></div><div><font face="monospace">response = manifests_api.read(ml_href, _preload_content=False)</font></div><div><font face="monospace">ml_dict = json.loads(response.data)</font></div><div><font face="monospace">ml = ContainerManifest(**ml_dict, local_vars_configuration=api_client.configuration)</font><br></div><div><font face="monospace"><br></font></div><div><font face="arial, sans-serif">The issue here is that in the serializer, the field config_blob is required, but there is also declared the additional field allow_null which is set to True. Yet, the api.json schema does not take that into consideration. Note that </font>bindings are generated from api.json schema. <font face="arial, sans-serif">The reason is </font>presumably <font face="arial, sans-serif">stated in the comment #7 here </font><a href="https://pulp.plan.io/issues/6069#note-7" target="_blank">https://pulp.plan.io/issues/6069#note-7</a>.</div><div><font face="arial, sans-serif"><br></font></div><div><font face="arial, sans-serif">And as you can see, the initial idea was to stray away from REST calls and to get rid of working with raw responses. However, it is inevitable even now in some use cases.</font></div><div><font face="arial, sans-serif"><br></font></div><div><font face="arial, sans-serif">I would love to see any follow-up discussion here because I believe that these little hacks I proposed can be problematic in the future. Still, we can wait for OpenAPI v3 where the issue with the field allow_null is resolved.</font></div></div> _______________________________________________<br> Pulp-dev mailing list<br> <a href="mailto:Pulp-dev@redhat.com" target="_blank">Pulp-dev@redhat.com</a><br> <a href="https://www.redhat.com/mailman/listinfo/pulp-dev" rel="noreferrer" target="_blank">https://www.redhat.com/mailman/listinfo/pulp-dev</a><br> </blockquote></div></div>