Comment 14 for bug 1282089

Revision history for this message
mouadino (mouadino) wrote :

   > keystoneclient.session.Session and requests.Session objetcs are not meant to be shared between different users.

AFAIK That's true for keystoneclien.session.Session but not always true for requests.Session and in our case i think that is ok because we are not using cookies for authentication.

  > However, it would make sense to have a global connection pool inside Horizon

With that you will break the abstraction layers, basically FMPOV the layers should like this

-----------------------------------------------
horizon
-----------------------------------------------
keystone client (token, catalog ...)
-----------------------------------------------
transport (http)
-----------------------------------------------

Now you don't want horizon to know if keystone is over http or plain tcp or kerberos or what ever, that's not horizon problem, so if we are going to manage connection pools we should do that in keystone client or not do it at all.

The more i think about this problem the more i believe that this is mostly due to a miss design in the keystone client, (i already mention that in my review patch and i will repeat here), i think the keystone.session.Session is doing more than it support to do, basically i think this class should manage a user session and not deal with http transport too (SRP).

I think a better design (to really fix this problem) will be to add another class which should manage endpoint sessions (EndPointSession), this class is the one that should end up dealing with HTTP requests so the keystone.session.Session will be keystone.session.UserSession and we should move methods like ``request``, ``_send_request`` and so on to the EndPointSession, and the trick will be to have one EndPointSession instance per endpoint, this way we will have one requests.Session() per EndPointSession instance which mean one connection pool per endpoint.

In code:

class EndPointSession(object):

      __endpoints = {}

      def __new__(cls, url, ...):
             return self.__endpoints.setdefault(url, super().__new__(cls, url, ...))

     def __init__(self, url, ...):
            self.session = requests.Session()
            ....

     def request(self, method, ...):
            self.session.request(...)

class Session(object):

      def __init__(self, ...):
             self.session = EndPointSession(url, ...)

Cheers,