[Pulp-dev] Bindings' limitations

[Dennis Kliban]

On Tue, Feb 11, 2020 at 6:04 PM Lubos Mjachky <lmjachky at redhat.com> wrote:

> Dear colleagues,
>
> 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.
>
> 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.
>
> 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:
>
> distribution_data =
> ContainerContainerDistribution(**gen_distribution(foo="bar")) # raised an
> exception; got an unexpected keyword argument 'foo'
> with self.assertRaises(ApiException) as exc:
>     self.distribution_api.create(distribution)
>
> But should do rather this:
>
> distribution_data = gen_distribution(foo="bar") # a simple dictionary
> with self.assertRaises(ApiException) as exc:
>     self.distributions_api.create(distribution_data)
>
> 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:
>
> configuration = Configuration()
> configuration.client_side_validation = False
> ContainerManifest(**manifest_without_required_field,
> local_vars_configuration=configuration)
>
> 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.
>
> 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:
>
> ml = manifests_api.read(ml_href) # raised an exception; invalid value for
> `config_blob`, must not be `None`
>
>
The problem here is that config_blob shouldn't be a required field. Let's
fix that in the serializer.


> Instead, you should do the following:
>
> response = manifests_api.read(ml_href, _preload_content=False)
> ml_dict = json.loads(response.data)
> ml = ContainerManifest(**ml_dict,
> local_vars_configuration=api_client.configuration)
>
> 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 bindings are generated from api.json schema. The
> reason is presumably stated in the comment #7 here
> https://pulp.plan.io/issues/6069#note-7.
>
> 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.
>
> 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.
> _______________________________________________
> 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/20200212/5b3b9d3b/attachment.htm>