[Pulp-dev] Bindings' limitations

[Lubos Mjachky]

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`

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