Azure / msrest-for-python

The runtime library "msrest" for AutoRest generated Python clients.
MIT License
41 stars 64 forks source link

Could we keep connection pool cross client? #133

Closed lmazuel closed 4 years ago

lmazuel commented 5 years ago

Today it's designed at best one client == one connection pool. Because one client == one requests.Session. Do we want to keep it cross-client?

@johanste FYI

See #132

lmazuel commented 5 years ago

It's complicated with the current architecture, because the HTTP layer is not shared at all. You can configure each client with different requests parameters. At the same time, it means you have to reconfigure each client (for example, if you have a proxy and 10 clients, you need to configure it 10 times). This leads to client factory as done in the CLI: a callback you use for each client because you cannot configure it at one:

for client in create_client():
   configure_client(client)
johanste commented 5 years ago

Being able to separate the connection (session/socket/whatever you want to call it) that holds the generic networking properties (e.g. read timeout, proxy configuration) - and that also would encapsulate the connection pool - could be interesting. The fact that the current requests.Session object also includes things like default HTTP headers (which, in our case, I believe may be client specific) could make it a bit more complicated.

lmazuel commented 4 years ago

In current azure-core, sharing the transport layer solves this. We won't do more in msrest