Comment 6 for bug 1282089

Revision history for this message
mouadino (mouadino) wrote :

@Florent: AFAIK Python know how to collect circular references unless you specify a __del__ method in one of them (http://docs.python.org/2/library/gc.html#gc.garbage) the GC may have to do more complex strategies to free up circular references (Which is of course not ideal) comparing to normal reference counter but it should be able to do that in the end, so we don't have any memory leaks from this. So i don't think adding a __del__ method will fix anything in the opposite it will introduce memory leaks.

@Florent: /For your first comment)

I still think that keystone client is miss-using the requests.Session this class is not meant to be instantiated more than once for a single endpoint, beside this i think that it will be good to take advantage also of the ability of using a connection pool, which is given to us for free by python-requests, Now you may say that may it's python-requests fault to not be that flexible so that we can disable connection pooling (Which is the main reason why connection are kept open) if we don't want to change keystoneclient, but wouldn't it be just better to use actually the connection pooling ?

As for horizon usage of keystoneclient the caching that you pointed our here (https://github.com/openstack/horizon/blob/master/openstack_dashboard/api/keystone.py#L164) is per request i.e. if a request is send from the browser to horizon this later will create a Request object and pass it to the target Django view and if the view must do more than one call to keystone, all of this calls should use the same keystoneclient.client.Client object (i.e. same token).