Traceback in debug mode due to pending aiohttp connections

[olitheolix]

From what I have found out so far this is because RESTClientObject relies on its __del__ method to be called before the event loop terminates. This is flakey since Python makes no guarantee to call that method at all.

A quick way to reproduce the problem on my Python 3.6 machine is to add this test to test_watch.py and run it in debug mode ie (export PYTHONASYNCIODEBUG=1):

    def test_foo(self):
        resource = kubernetes_asyncio.client.CoreV1Api().list_namespace
        assert False

This produces a lengthy stack trace, but the salient part is this:

Unclosed client session
client_session: <aiohttp.client.ClientSession object at 0x7f140e947d30>
source_traceback: Object created at (most recent call last):

...

  File "/home/oliver/pocs/kubernetes_asyncio/kubernetes_asyncio/watch/watch_test.py", line 26, in test_foo
    resource = kubernetes_asyncio.client.CoreV1Api().list_namespace
  File "/home/oliver/pocs/kubernetes_asyncio/kubernetes_asyncio/client/api/core_v1_api.py", line 33, in __init__
    api_client = ApiClient()
  File "/home/oliver/pocs/kubernetes_asyncio/kubernetes_asyncio/client/api_client.py", line 70, in __init__
    self.rest_client = rest.RESTClientObject(configuration)
  File "/home/oliver/pocs/kubernetes_asyncio/kubernetes_asyncio/client/rest.py", line 81, in __init__
    connector=connector

The problem does not materialise if I change

resource = kubernetes_asyncio.client.CoreV1Api().list_namespace

to

kubernetes_asyncio.client.CoreV1Api().list_namespace

because __del__ will be called immediately, presumably because Python garbage collects the object immediately since nothing ever references it.

As a first attempt to address the problem I added an explicit shutdown method to RESTClientObject
and all the classes that instantiate it up to Watch. Users must then explicitly call
Watch().stream(...).shutdown() or use the context manager I created for it. This feels inelegant and is a maintenance burden. Ideally, we could register a callback in RESTClientObject that triggers whenever the event loop shuts down because that is when we need to close pending connections. Any ideas?

I should probably point out that users will not see the stack trace unless they run in debug mode, but I would still like a clean solution for this because I am petty 😄

[tomplus]

Thanks! I like your idea of using the context manager or shutdown method. It will be predictable and give a possibility to control closing connections. So, it has to be done in swagger-codegen first and adopted here.

__del__ is not the best solution for sure, but it is simple and works quite well. The problem is with ending programs when we these methods may not be called at all or may be called in random order by garbage collector. I tried to create situation where it doesn't work (a lot of object, asynchronous calls etc.) but I didn't find any problems. BTW, this error from aiohttp about 'unclosing connections' comes from __del__ method too.

[olitheolix]

The problem is with ending programs when we these methods may not be called at all or may be called in random order by garbage collector.

Correct. I agree it is not a show-stopping problem but needs fixing regardless.

It has to be done in swagger-codegen first and adopted here.

Yes. I hacked it into the generated classes to proof this would work but I have never used swagger-codegen before. Do you have an ETA for this, by chance?

[mickours]

The problem is with ending programs when we these methods may not be called at all or may be called in random order by garbage collector.

Correct. I agree it is not a show-stopping problem but needs fixing regardless.

It has to be done in swagger-codegen first and adopted here.

Yes. I hacked it into the generated classes to proof this would work but I have never used swagger-codegen before. Do you have an ETA for this, by chance?

Did you manage to make this work? I'm really interested in a graceful shutdown method. I'm not sure to understand. Does this depends on an external project API change or can be done in this one?

[tomplus]

@mickours Could you show your use case? It it related to Watch or Stream ?

[tomplus]

The issue is already fixed in 11.2.0. There are the context manager and the close() method to explicit close client connections.