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

Incomplete async support: sync_to_async usage in ModelService #215

Closed
CMarcher-Ahuora opened this issue Dec 6, 2024 · 2 comments · Fixed by #216
Closed

Incomplete async support: sync_to_async usage in ModelService #215

CMarcher-Ahuora opened this issue Dec 6, 2024 · 2 comments · Fixed by #216
Labels
bug Something isn't working

Comments

@CMarcher-Ahuora
Copy link

Within the ModelService class, the "asynchronous" object operations appear to just use the sync_to_async wrapper function provided by Django, while ultimately calling a synchronous Django ORM function.

For example, the get_one_async function calls get_one with sync_to_async:

    async def get_one_async(self, pk: t.Any, **kwargs: t.Any) -> t.Any:
        return await sync_to_async(self.get_one, thread_sensitive=True)(pk, **kwargs)

And get_one calls a synchronous in-built Django function get_object_or_exception.

   def get_one(self, pk: t.Any, **kwargs: t.Any) -> t.Any:
        obj = get_object_or_exception(
            klass=self.model, error_message=None, exception=NotFound, pk=pk
        )
        return obj

Asynchronous versions of many of these ORM functions, such as get_object_or_exception are available (aget_object_or_404). Another resource showing async used within core Django features: async queries. Where possible, current use of synchronous ORM functions should be converted to their younger asynchronous counterparts, allowing django-ninja-extra to have proper async support.

@eadwinCode
Copy link
Owner

eadwinCode commented Dec 9, 2024

@CMarcher-Ahuora Oh that was an oversight. And thanks for the suggestion. Will look into this soon

@eadwinCode eadwinCode added the bug Something isn't working label Dec 9, 2024
@CMarcher-Ahuora
Copy link
Author

To be fair to you, I have only just learned that all async Django ORM functions are just wrappers for sync_to_async as well, so I'm not sure whether it would be worth going through the effort of using these "async" versions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants