(Re)introduce Async Client Implementation

[micha91]

Since the introduction of the async/await pattern in Python 3.5, more and more open source tools are using async implementations. This is also the case for Music Assistant, where Deezer is already integrated using this library in Music Assistant 2 beta. Since Music Assistant needs to handle many requests in a highly performant way, @Un10ck3d implemented an async version of this library in a fork. We believe it would be better to provide both the sync and async implementations of the client in one project. This would reduce the overall maintenance required to keep both repos up to date, and people would not have to choose between two libraries.

There are several ways to achieve this goal.

Both implementations in one Client class

We could simply introduce an async version for each function. To avoid doing all the work twice, we could implement an additional function that generates the URL, params, and (if needed) body for the request. For example

    def _get_tracks_chart_args(self, genre_id: int = 0) -> tuple:
        return "GET", f"chart/{genre_id}/tracks"
    
    async def async_get_tracks_chart(self, genre_id: int = 0) -> list[Track]:
        """
        Get top tracks for the given genre ID.

        :param genre_id: the genre ID, default to `All` genre (genre_id = 0).
        :return: a list of :class:`~deezer.Track` instances.
        """
        return await self.async_request(*self._get_tracks_chart_args(genre_id))
    
    def get_tracks_chart(self, genre_id: int = 0) -> list[Track]:
        """
        Get top tracks for the given genre ID.

        :param genre_id: the genre ID, default to `All` genre (genre_id = 0).
        :return: a list of :class:`~deezer.Track` instances.
        """
        return self.request(*self._get_tracks_chart_args(genre_id))

In addition, we would also implement an AsyncPaginatedList to provide a high performance async iteration. The aiohttp client could also be initiated in the async_request function on the first call, if it is not already available. This way, we would not carry an aiohttp client for those, who only want to use the the sync implementation.

An AsyncClient and a Client class

Alternatively, we could create an AbstractClient that contains all the URL, params and body generation, and then derive the Client and AsyncClient from it:

class AbstractClient(abc.ABC):
    def _get_tracks_chart_args(self, genre_id: int = 0) -> tuple:
        return "GET", f"chart/{genre_id}/tracks"

class AsyncClient(AbstractClient):
    async def get_tracks_chart(self, genre_id: int = 0) -> list[Track]:
        """
        Get top tracks for the given genre ID.

        :param genre_id: the genre ID, default to `All` genre (genre_id = 0).
        :return: a list of :class:`~deezer.Track` instances.
        """
        return await self.request(*self._get_tracks_chart_args(genre_id))

class Client(AbstractClient):
    def get_tracks_chart(self, genre_id: int = 0) -> list[Track]:
        """
        Get top tracks for the given genre ID.

        :param genre_id: the genre ID, default to `All` genre (genre_id = 0).
        :return: a list of :class:`~deezer.Track` instances.
        """
        return self.request(*self._get_tracks_chart_args(genre_id))

In this case, we should also implement classes that provide methods like Artist in sync and async to not mix the patterns here.

How does the community feel about this? Do you prefer one of these solutions or do you have another approach? And most importantly: Do you think there are good reasons to include an async implementation or should it not be part of this library? Thanks in advance for a good discussion 👍

[browniebroke]

Thanks for raising this. I've been thinking about implementing an async/await based client since removing the tornado-based async client, but I never got around to put the work in for it 😄

I recently noticed the async fork and wanted to suggest bringing it back here, but as you say, there are a few ways we could go about it. I've been watching what happens in the Python ecosystem, I know of 2 examples which have taken a variation of the approach you suggest:

1. Django has been adding async support to various part of the stack over the past few years: middleware, views, ORM, etc... Here is the technical document describing the approach. The changes added to the ORM in particular duplicated the existing methods with an a prefix: get() -> aget(), create -> acreate...
2. httpx, a modern requests replacement with async support provides 2 Client classes: one sync and one async. I don't know much about the internal implementation details, but would be worth a look at how they've done it.

The function "colouring" (sync vs async) is what bothers me the most. It seems like, either way, we'll have to duplicate a lot of code. There is obvious duplication in the Client, but in our case, this ripple down the resources as well.

I'd like to suggest a 3rd -more radical- approach: go full async, as new major version.

This library is dealing with network, where async/await is a great choice. Modern Python is going more and more async, after all. If folks don't want async, they can either stay on an older version, before the major bump, or use an async_to_sync wrapper.

[micha91]

Yeah, @Un10ck3d and I agreed that we want the async version asap to not block our further development in Music Assistant, but also wanted get in touch with you right after we have the first implementation to share our work with the community here.

I am already using httpx in one of my projects at work, because it is used by the openapi-python-client. From my experience you still need to create a sync and an async client individually and have to await functions of the async client. For that reason, I wasn't able to identify a big upside of using httpx - except for the common API, the two clients share. But as far as I know, we would still have to implement both, sync and async implementations, if we wanted to support both. For performance reasons, I would highly prefer aiohttp for the time being as it is way faster on load as this comparison shows.

Regarding the naming of functions in a hybrid client, I would be totally okay with following the naming convention of Django - the async_ prefix is being used in the Home Assistant project and for that reason it came to my mind, but as this library is meant to be used be developers and Django is a pretty common web framework in the python context, it would maybe be better to use their naming.

Regarding your 3rd approach - from our point of view, this would all be fine, since we are async users. But I don't know how the rest of the community works. Especially in the scripting area, I don't think there are many people using async code, and in most cases they don't care that much about performance. Python is still and will always be a language used not only by professional developers, but also by people who just want to tinker around.

[micha91]

Okay, we now had given it almost 3 months to get feedback and it seems like there is not to much coming in 😅

I would propose to completely move to async and use a wrapper like this one. I never used it, but it looks pretty promising and appears to have support for AsyncGenerators. Also I would propose to use aiohttp for performance reasons. If there is nothing against it, I would offer to rebase the the async fork, implement a real async PaginatedList and add the async wrapper to all classes. Then I would check the tests and try to figure out, how we best test that all stuff still works in sync as expected. Afterwards I would open a PR and there we could go into the details. Does that sound like a good path forward for you? 🙂