HENNGE / aiodynamo

Asynchronous, fast, pythonic DynamoDB Client
https://aiodynamo.readthedocs.io/
Other
69 stars 20 forks source link

Session instantiation #147

Closed torphix closed 1 year ago

torphix commented 1 year ago

Hi thanks for the cool lib!

With regards to starting a session i noticed in the documentation you state that you want to create sessions as little as possible.

Therefore my approach has been to create a session once upon the starting of my server like so:

async def initialize_db() -> Client:
    session = ClientSession()
    client = Client(AIOHTTP(session),
                        StaticCredentials(Key(id=settings.AWS_ACCESS, secret=settings.AWS_SECRET)),
                        settings.AWS_REGION,
                        yarl.URL('http://127.0.0.1:8000') if settings.debug_mode else yarl.URL(settings.AWS_DYNAMO_LINK))
    return client

app = FastAPI(dependencies=[])

@app.on_event("startup")
async def startup_event():
    globals = Globals.instance()
    globals.db = await initialize_db()

With globals acting as a singleton class that can therefore be used throughout the app.

However this does mean that the session never gets closed manually.. is this a problem?

Would it be better to instantiate a new session every time I need to make queries to the database?

dimaqq commented 1 year ago

You can totally do that. I’ve done it. It’s particularly meaningful in case of FastAPI, as there were issues with starlette and async contexts.

Your code as stated may be enough if your app never exits, eg runs until container is killed.

On the other hand, if your app is instantiated many times in tests, you can add a shutdown hook too to make the lifecycle cleaner.

ojii commented 1 year ago

Therefore my approach has been to create a session once upon the starting of my server like so:

your approach is similar to the one we use with fastapi, though we explicitly close the http session on app shutdown.

Would it be better to instantiate a new session every time I need to make queries to the database?

That would be worse. You could create aiodynamo.client.Client instances for each query if for some reason you want to do so, but the http session and (non-static) credentials instances benefit from being long lived. For http sessions it's so the http client can have a connection pool to re-use and for credentials it's to cache credentials if they're not static.

dimaqq commented 1 year ago

P.S. @torphix what kind of environment does your code run in? If it's a lambda, then a different set of concerns applies.

torphix commented 1 year ago

Thanks for getting back to me :) The environment is indeed using AWS lambda + Mangum + AWS API gateway What sort of alterations / design patterns changes should be adhered to?

dimaqq commented 1 year ago

That has it's own set of issues, incl. https://github.com/jordaneremieff/mangum/issues/208 ➡️ https://github.com/jordaneremieff/mangum/discussions/211 in short, watch out for your on_event("startup") actually getting called for every request.

Basically you want to prioritise fast startup over everything else, and even then possibly use the lifecycle hooks.

Perhaps a global singleton is better... but then if lambda is suspended and resumed you may get errors or slowdown if the dynamodb key has expired... ultimately you probably need to use the lifecycle hooks 🤷🏻

torphix commented 1 year ago

Thanks!