#658: fix for async Auth #756
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
|
I've checked it again, so re-open this PR. |
|
Hi mantainers, |
There was a problem hiding this comment.
I was using the decorator to workaround this issue:
def await_auth(f):
@functools.wraps(f)
async def decorator(*args, **kwargs):
auth = await args[0].auth
if not auth :
raise AuthenticationError()
args[0].auth = auth
return await f(*args, **kwargs)
return decorator
But it seems these PR fix this async auth issue, nice work!
|
@vitalik Are there any async auth fix foreseen for the next release? |
ninja/operation.py
Outdated
| if asyncio.iscoroutinefunction(callback): | ||
| result = async_to_sync(callback)(request) | ||
| elif isinstance(callback, AuthBase) and asyncio.iscoroutinefunction(callback.authenticate): | ||
| result = async_to_sync(callback)(request) | ||
| else: | ||
| result = callback(request) |
There was a problem hiding this comment.
use asgiref.iscoroutinefunction :)
It provides a backport of this functionality
There was a problem hiding this comment.
@baseplate-admin
you mean asgiref.sync.iscoroutinefunction ?
|
@SmileyChris Thank you! |
|
Thank you reviewers ! |
|
to check for async - use signature.utils.is_async which should be compatible with 3.7 |
Is it not better to use a asgiref function ? well it break tests :) btw - check #735 as well - this seems already all tests covered |
|
Besides @vitalik, your function is deprecated. Check : python/cpython#94912 ( maybe we should switch to asgiref one everywhere ? ) |
|
@baseplate-admin @vitalik try:
import asgiref
iscoroutinefunction = asgiref.sync.iscoroutinefunction
except ModuleNotFoundError:
# For Django 2.x
import asyncio
iscoroutinefunction = asyncio.iscoroutinefunction |
well utils.is_async should be then adapted anyway - as this function is used in multiple places it just good practice to reuse the same code |
Thats exactly what asgiref is doing. Except it imports from inspect.
Would you like a PR to remove asyncio.iscoroutinefunction? |
|
So then it's better to use |
|
Oops, didn't notice this #756 (comment) |
Right |
|
updated 15274a0 |
|
@skokado looks ninja.pagination is broken class PaginationBase contains a queryset sync method called _items_count() Any sugestions? |
@luizfelipevbll How can I see the behavior of that? pytest
|
|
Hi reviewers, on second thought, this update may be fundamentally misguided :( https://github.com/vitalik/django-ninja/actions/runs/4894118600/jobs/8750675243?pr=756#step:6:30 it's used Should I wait for Django 2 to drop? #744 |
I thought of this too when i closed my own PR. see #719 (comment) and i then made my own middleware package to extend |
|
Hi, thank you for contribution, but another PR with async auth merged, please test with latest master branch |
Fix #658 the async-auth behavior
Fix the behavior not to be completed the auth callback.
see below
#658 (comment)