Skip to content

[REQ] Python: improve the typing of API clients #16788

@multani

Description

@multani

Is your feature request related to a problem? Please describe.

In #16694, I tried to add typing information on the generated API clients, but the issues are a bit larger than expected.

Going from bottom (REST client) to up (the model API client):

REST API client

By "REST API client", I mean the RESTClientObject object.

The REST API client request() method can return very different type of responses:

  1. A basic ApiResponse, a generic HTTP response
  2. Any kind of higher-level object via response_types_map: the client can extract the payload of the response and deserialize it into any kind of object if the payload matches the format of that object (so, an int, datetime, or any model; but actually, almost any type)

This has several issues:

  • This high variability in the kind of responses the client can return makes it very difficult to type correctly.
  • The different response types change depending on parameters passed to the REST client. This makes the typing even more difficult to implement and pushes all the type verification to the client of the object.
  • Finally, it adds the higher-level deserialization logic of the response into the REST Client, which could be seen more as a low-level client.

Model API client

By "Model API client", I mean the API client created for each model.

I'm taking a hypothetical Foo model with basic CRUD methods as an example, that would produce a FooApi class.

We can find the following kind of methods:

  1. A get_foo_by_id method returning:
    1. Foo (usual case)
    2. None in some cases, sometimes for errors too
  2. A get_foo_by_id_with_http_info, which could return either:
  • An ApiResponse: a generic HTTP response
  • An instance of the associated Foo model, if _return_http_data_only is False
  1. There are other methods that can return more complex objects, like List[Foo], etc. but the mechanism is the same as explained above.

In addition, the urllib3 client can also produce an AsyncResult with one of the 2 behaviors explained just above.
This behavior doesn't exist for the aiohttp-based client.

This also has several issues, similar to those of the underlying REST Client explained above:

  • This high variability in the kind of responses the client can return makes it very difficult to type correctly.
  • The behavior can change drastically, depending on the parameters passed
  • The deserialization logic is done in the REST Client, although it's only (mostly) used in the high-level APIs get_foo_by_id and similar.
  • The AsyncResult responses create additional complexity in the typing of the responses, since it also depends on the parameter async_req to be set.

Describe the solution you'd like

After some good suggestions from @fa0311, @robertschweizer and @wing328 , I'd like to make the propose the following changes:

Remove all the parameters that change the response type and move the associated behaviors somewhere else:

Remove _return_http_data_only

Remove the deserialization behavior away from the RESTClientObject class and move it into the xxx() methods of the model API client:
1. The xxx_with_http_info() methods would not be able to deserialize the response to high-level objects anymore
2. The xxx_with_http_info() methods would only return ApiResponses object instead
3. The deserialization logic would be moved outside of the REST Client:
1. It actually doesn't have any state and could be free functions instead.
2. This can all be moved to a dedicated deserialization module.

Remove async_req

Extract the AsyncResult into dedicated *_async() methods.

  1. This means that, for each xxx() + xxx_with_http_info() methods, this would add 2 new methods:
    1. xxx_async()
    2. xxx_with_http_info_async()
  2. The behavior of the 2 new methods would be the same as the original methods, except the processing of the response would be done only when the actual result is fetched from the AsyncResult object.
  3. This would require to refactor a bit the code to make these behaviors reusable beteen xxx()/xxx_async() and xxx_with_http_info()/xxx_with_http_info_async():
    1. The deserilization logic for a xxx() method would be moved into a _xxx_deserialization() method
    2. xxx() would call this method directly before returning its result
    3. xxx_async() would call this method when the result is fetched.

Create a new ApiRequest object

Finally, to makes it easier to reuse the code between the sync/async methods, I'd like to introduce a new ApiRequest object:

  1. The behavior from the *_with_http_info() would be moved to a dedicated "private" method
    1. This method would take all the arguments and build a new ApiRequest object containing the right parameters to execute the request
    2. This method could be called both by the xxx_with_http_info() and the xxx_with_http_info_async() methods
  2. The REST Client, instead of taking many parameters, now only takes the ApiRequest object and uses whatever it needs from that.

Additional context

These changes are not really backward compatible. All the parameters removed will not be supported anymore.

Each changes could be done separately, although the typing benefits would only be seen once all the changes have been done
Ideally, the best order would be:

  • introducing the new ApiRequest object and use it both from the *_with_http_info() methods and the REST Client. The REST Client can be kept backward compatible for the time being.
  • async_req can be extracted then be extracted. This simply duplicates the methods, removes the parameter and reuses the ApiRequest created above.
  • The deserialization logic can finally be moved and the typing simplified

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions