altdesktop / python-dbus-next

🚌 The next great DBus library for Python with asyncio support
https://python-dbus-next.readthedocs.io/en/latest/
MIT License
187 stars 59 forks source link

Question: Asynchronous properties? #86

Closed sphh closed 3 years ago

sphh commented 3 years ago

I want to wed python-dbus-next with mopidy-async-client. Since mopidy-async-client is asynchronous, I have a couple of async methods in my interface. That works fine.

When the service comes available, the sound applet is checking for its capabilities, e.g. it requests SupportedUriSchemes, which I implemented in the dbus-next interface. The problem I have, is that some of these property getters (and some setters) also use asynchronous functions of the mopdiy-async-client, e.g.:

class MopidyInterface(dbus_next.service.ServiceInterface):
    def __init__(self, mopidy):
        super().__init__('org.mpris.MediaPlayer2.Player')
        self.mopidy = mopidy

    @dbus_property(access=PropertyAccess.READ)
    def SupportedUriSchemes(self) -> 's':
        schemes = self.mopidy.core.get_uri_schemes()
        return schemes

where .get_uri_schemes() is an asynchronous method. So this does not work, because it would require an await. The error message is:

/usr/local/lib/python3.8/dist-packages/dbus_next/message_bus.py:230: RuntimeWarning: coroutine 'CoreController.get_uri_schemes' was never awaited
  body[interface.name][prop.name] = Variant(prop.signature,
RuntimeWarning: Enable tracemalloc to get the object allocation traceback

If I change it naïvely to

    @dbus_property(access=PropertyAccess.READ)
    async def PlaybackStatus(self) -> 's':
        state = await self.mopidy.playback.get_status()
        return state.capitalize()

I get the very same error.

Is there a way to have asynchronous properties?

acrisci commented 3 years ago

I don't believe this is supported right now, but this would be an excellent feature for the library to have.

sphh commented 3 years ago

Is that difficult to implement?

acrisci commented 3 years ago

No this wouldn't be difficult. Async service methods are already supported and properties are just like that. I think it would be easy as checking if the getter or setter definition is a coroutine and going down that path instead of just getting or setting the property.

sphh commented 3 years ago

I would like to help with a PR, but having looked into the code, that's unfortunately above my pay grade …

acrisci commented 3 years ago

Ok since the work is clear and I think it's an important feature, I should get around to it soon.

sphh commented 3 years ago

Just out of curiosity: Will I then be able to use that property from another method, e.g. print_schemes below without any side-effects:

class MopidyInterface(dbus_next.service.ServiceInterface):
    def __init__(self, mopidy):
        super().__init__('org.mpris.MediaPlayer2.Player')
        self.mopidy = mopidy

    async def print_schemes(self):
        schemes = await self.SupportedUriSchemes
        print(schemes)

    @dbus_property(access=PropertyAccess.READ)
    async def SupportedUriSchemes(self) -> 's':
        return await self.mopidy.core.get_uri_schemes()

or do I have to resort so this pattern:

class MopidyInterface(dbus_next.service.ServiceInterface):
    def __init__(self, mopidy):
        super().__init__('org.mpris.MediaPlayer2.Player')
        self.mopidy = mopidy

    async def print_schemes(self):
        schemes = await self.get_supported_uri_schemes()
        print(schemes)

    async def get_supported_uri_schemes(self):
        return await self.mopidy.core.get_uri_schemes()

    @dbus_property(access=PropertyAccess.READ)
    async def SupportedUriSchemes(self) -> 's':
        return await self.get_supported_uri_schemes()

(I want to prepare my code as far as possible for these amazing asynchronous properties!)

acrisci commented 3 years ago

I'm not sure how the language handles async getters, but I know it's not supported with normal properties. So async property accessors probably won't have all the features. The second way is what I'm aiming for.

acrisci commented 3 years ago

Let me know if that works.

sphh commented 3 years ago

Wow, that's more than a simple change. Chapeau!

Will you publish it on pypi or do I have it to install it from github (not a problem, just a tiny bit of work – definitely less than your changes!!). I'll test it in the next days.

You write:

When aio.MessageBus is used, property getters and setters can be coroutines, although this will cause some functionality of the Python @property annotation to be lost.

Can you give some quick hints, what functionalities will be lost?

acrisci commented 3 years ago

You will not be able to access it as a property, only use the getters and setters.

sphh commented 3 years ago

So far so good!! It works.

I have just one question: Can I mix'n'match? Can I have a @dbus_property with an async getter and a simple setter? Or a simple getter and an async setter? Or do I have to match both to be either simple or async?

acrisci commented 3 years ago

You can mix them and it should be fine.

sphh commented 3 years ago

Great. That's what I did, but I just wanted to be sure, if there are no side effects.

It works perfectly in my application!! Thank you so much.