Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Core client #6

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Core client #6

wants to merge 2 commits into from

Conversation

Andrew-Chen-Wang
Copy link
Owner

@Andrew-Chen-Wang Andrew-Chen-Wang commented Jan 6, 2023

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: django/django#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.

@Andrew-Chen-Wang Andrew-Chen-Wang changed the base branch from main to new-redis-package January 6, 2023 01:52
Base automatically changed from new-redis-package to main January 10, 2023 19:31
Signed-off-by: Andrew-Chen-Wang <acwangpython@gmail.com>
Signed-off-by: Andrew-Chen-Wang <acwangpython@gmail.com>
@smallfish06
Copy link

waiting for minor release 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants