Andrew-Chen-Wang / django-async-redis

Full featured async Redis cache backend for Django.
Other
22 stars 4 forks source link

Core client #6

Open Andrew-Chen-Wang opened 1 year ago

Andrew-Chen-Wang commented 1 year ago

TL;DR I support two separate classes to enforce standardization of libraries knowing whether a sync context is available to execute.

I'm now reconsidering the approach back to using a single class. Although the options will be confusing in the constructor, the Django core approach has been to use an a prefix or ASYNC in string names.

In terms of functionality, if we think of a typical library like django-cachalot, then which cache does the library use? If a cache only supports async, how would cachalot know its sync counterpart to execute in sync form?

This is important to know in the case of context switching within Django itself (e.g. async middleware vs. sync view or the view is simply calling something that is sync).

In the Django async database engine approach I've done, we have an option in the settings that specifies the sync counterpart's DATABASES alias (ref: https://github.com/django/django/pull/15357) in order to perform migrations (since async does not support migrations). Would we do the same here if we create two separate classes? Especially considering that many database/cache libraries don't include their counterpart environment (e.g. aioredis only does async and psycopg2 only does sync I think).

The better expectation is to specify a sync cache alias in the async portion. Because a project is typically only in one environment (ASGI vs. WSGI), specifying a "sync" counterpart if a dev's project's context is async would make a standardized approach for libraries to know whether a specific environment is supported.

And perhaps another option could even be to have the SYNC option be given "self" which can mean a class supports both sync and async, but bottom line: sync option specificity is required.

smallfish06 commented 1 year ago

waiting for minor release 👍