openapi-generators / openapi-python-client

Generate modern Python clients from OpenAPI
MIT License
1.28k stars 197 forks source link

Removal of an unused deprecated field causes errors #1019

Closed WillDaSilva closed 6 months ago

WillDaSilva commented 6 months ago

I was using a generated Python client for an API that had a deprecated password field. This generated client was being used in our production environment. Then the API removed the deprecated password field. We expected that this would have no effect on us because we weren't using it.

Unfortunately it made it so that our Python client for the API could no longer deserialize the response into the model for it, so when we accessed the host field of the response (i.e. response.host) an AttributeError was raised. This is because when the generated Python client fails to deserialize a response into a model, it returns a dictionary instead. The dictionary had the host field we were trying to access, but it would need to be accessed via __getitem__, i.e. response['host'] instead of response.host. This caused an outage in production until we updated our Python client to expect the new response that lacked the password field.

We're now trying to find a way to avoid this sort of outage in the future. One way I can think to avoid this problem would be to have the generated client do something like so:

def __getattr__(self, key: str):
    try:
        return self.__getattribute__(key)
    except AttributeError as ex:
        try:
            return self.__getitem__(key)
        except KeyError:
            raise ex

Using response.host as an example, this would first try to access the field normally, response.host, and then fall back to response['host'] if response.host raised an AttributeError. If the fallback doesn't work, the original exception is raised.

Along with an approach like this, logging a warning that the response could not be deserialized would be valuable.

If something like this were implemented, then instead of our system experiencing an outage when the API we rely on removed a field, we would have stated to see warnings that the API client is failing to deserialize the responses. Then we could've updated the client to reflect the removed field, and experienced no downtime.