Closed chrysn closed 3 years ago
Mic92
commented
3 years ago Do you also plan to fix?
/home/runner/work/python-mpd2/python-mpd2/mpd/asyncio.py:219: DeprecationWarning: The loop argument is deprecated since Python 3.8, and scheduled for removal in Python 3.10.
chrysn
commented
3 years ago Thanks, missed that; WIP together with the suggested name changes.
chrysn
commented
3 years ago Next steps from me are looking into another warning that's raising (it's really coming out of the mocker tester, but still...).
One more step that could be taken in here is to do the #121 changes right away, for which I think I already have a half-baked proposal. The only reason to keep it in here though is that it'd revert / simplify some of the binary changes in this PR.
Mic92
commented
3 years ago Next steps from me are looking into another warning that's raising (it's really coming out of the mocker tester, but still...).
One more step that could be taken in here is to do the #121 changes right away, for which I think I already have a half-baked proposal. The only reason to keep it in here though is that it'd revert / simplify some of the binary changes in this PR.
Maybe I should also remove the deprecated api (send_idle etc) so that the test logs are cleaner w.r.t. to deprecation messages.
chrysn
commented
3 years ago If the send/fetch pairs were to go away, that might make the changes to have _execute_binary more aligned with _execute easier; in this case, I'd rather not do the #121 changes in this PR but, pending your approval (upon which I'd squash in the fixups), when both this and the removal of the deprecated functions is through.
Mic92
commented
3 years ago If the send/fetch pairs were to go away, that might make the changes to have _execute_binary more aligned with _execute easier; in this case, I'd rather not do the #121 changes in this PR but, pending your approval (upon which I'd squash in the fixups), when both this and the removal of the deprecated functions is through.
_send/_fetch is removed.
chrysn
commented
3 years ago This has been incorporated into #141 and approved there (with further commits squashing into those here); therefore, closing.
Apart from small fixes, this brings two changes:
An overhaul of the backend, which now tries not to eagerly read lines any more and feed them to whichever command is around, but hands control to the command to drive extraction on its own.
The new design also allows commands to be sent at invocation time, meaning that consecutive commands only take multiples of the round-trip time if they are awaited individually before sending the next command.
Added support for binary data (which required a change in the style of the above) closing #138.
Tests were added for the new functionality.
One test was changed, which in theory indicates potential for a breaking change. As the breakage was just in moving the exception from the creation of the future to its awaiting, it's unlikely to hit user code -- especially given that the actual code is the one that's just testing the asynchronous mocker.
One item possibly worth discussing is the binary extraction process. Right now, this is following precisely what the synchronous one does for
albumart: just produce the binary and ignore any other keys. In light of #121, that may be worth reconsidering.