ApiClient.deserialize should be a function, not a method

[horazont]

As far as I can tell, it does not use any attributes of the ApiClient.

The advantage of having it as function would be to have it available without needing to construct an ApiClient (as the Watch currently does). Normally, constructing an ApiClient would not be a problem. However, if one is in a non-async context, one cannot properly clean the client up and that will cause a warning ("asyncio: ERROR: Unclosed client session").

I would be happy to provide a PR if you agree.

[tomplus]

Hi @horazont

I'm afraid this refactoring won't be easy. Serialize() calls __serialize() which uses some class attributes: https://github.com/tomplus/kubernetes_asyncio/blob/master/kubernetes_asyncio/client/api_client.py#L304 And what's more it's auto-generated by openapi-generator so we have to change the generator first.

Other solutions which you can consider:

1. Use context manager with ApiClient or await ApiClient.close() after deserialization.

2. If it's safe, disable aiohttp warning (configure logging) 😈

3. Use https://github.com/kubernetes-client/python - it's synchronous version of this library

[horazont]

Aha, I see. I scanned the methods twice for references to self beyond the calls to __deserialize_* and missed that. Sorry.

Yet, this would still work just the same if the methods were @classmethod, which had about the same effect. How hard would it be to teach that to the generator? Easier than breaking it out in a separate function/module I expect.

As a workaround, we’re currently doing:

    api_client = ...
    try:
        # do things
        pass
    finally:
        asyncio.create_task(api_client.close())

:-X

Terrible enough, but also shuts up the warning. We don’t want to silence it globally because we had cases of api_client leaks in the past and we want to learn about them. We also don’t want to load both kubernetes_asyncio and kubernetes, and we need kubernetes_asyncio for it being asyncio :).

The root issue we’re trying to solve is that we need to write a function which inspects V1PodSpec and it needs to deal with both the raw JSON representation and the V1PodSpec instances (depending on where the data comes from). As .as_dict() on the V1PodSpec does not recreate the JSON representation correctly (for example, volume_mounts vs. volumeMounts), we went the other way. If there’s a nicer solution to this, that’d also be appreciated.

[tomplus]

Changing generator is not an easy, especially if you want to be backward compatible.

Do you consider using attribute_maps ? Each model has it. For instance: https://github.com/tomplus/kubernetes_asyncio/blob/master/kubernetes_asyncio/client/models/v1_pod_status.py#L51

I think, we can add a simple function to module "utils" which takes object as input and returns dict using this map. What do you think?

[horazont]

That would effectively be a re-implementation of what __serialize does, which I suppose would be fine.