meraki / dashboard-api-python

Official Dashboard API library (SDK) for Python
MIT License
289 stars 151 forks source link

Question about Async session re-use #171

Closed zabrewer closed 2 years ago

zabrewer commented 2 years ago

@coreGreenberet

so I have a design pattern that is an abstraction layer on top of the async code (basically makes it easier for people to implement async code and scripts).

In the example code you comment that the async “with” is required. For the implementation of the design pattern I have in mind I’d like to reuse the async connection.

In your testing do you have a good way to control the costly TCP connection to the dashboard API using async and re-use it or would this require a new design for the underlying SDK.

thanks!

coreGreenberet commented 2 years ago

the "async with" statement is required, so that the underlying session objects & connections will be correctly closed. You can get rid of the "async with" and just await __aenter__() and __aexit()__ directly instead. In the backend the default settings of aiohttp.ClientSession with the default aiohttp.TCPConnector are getting used. At the moment there are no parameters to change these settings directly. However this is still python, so you could overwrite the underlying objects, if you like.

Are you really worried about the TCP connection or the requests per second API limit? The library uses a asyncio.Semaphore in the backend to make sure, that you are not exceeding X concurrent requests to the dashboard. It's not like the token bucket model on the server side based on the org, but it helps to stay under the limit.

class AsyncDashboardAPI:
    """
    **Creates a persistent Meraki dashboard API session**
.....
    - maximum_concurrent_requests (integer): number of concurrent API requests for asynchronous class
.....
    """

You can use this setting to change the allowed concurrent requests per meraki object (default=8)

zabrewer commented 2 years ago

Thanks @coreGreenberet for the quick reply! I will take a look and get back to you. One thing I thought about was a try/except/finally where the finally closed the session but after reading this, I need to dig deeper into the backend.

TKIPisalegacycipher commented 2 years ago

Hi @zabrewer do you see any further cause for concern here?

zabrewer commented 2 years ago

@TKIPisalegacycipher - I've been too busy with other projects to follow up with this you can close this issue and I will tackle it ASAP.