irmen / Pyro5

Pyro 5 - Python remote objects
https://pyro5.readthedocs.io
MIT License
311 stars 36 forks source link

Reasons for removing the async proxy code #98

Open pevogam opened 3 days ago

pevogam commented 3 days ago

Hi, we were almost done migrating our use cases from Pyro4 to Pyro5 when I noticed this

def asyncproxy(*args, **kwargs):
    raise NotImplementedError("async proxy is no longer available in Pyro5")

I traced back the history to read some motivation on why this was removed in the git history but all I could find is

Author: Irmen de Jong <irmen@razorvine.net>  2017-03-12 15:41:32

    removed all async proxy code

and

Author: Irmen de Jong <irmen@razorvine.net>  2019-04-16 23:30:42

    async proxy isn't going to come back any time soon

without any clue why a piece of code which was already existing and extending the current functionality is gone. The modified TODO had "create an alternative for Pyro4's async proxy, and fix the examples that use it: distributed-computing2, distributed-mandelbrot" but no clear direction why an alternative was needed on the first place. The Upgrading from Pyro4 section also doesn't mention anything.

irmen commented 3 days ago

The upgrading from Pyro4 does mention it though:

It says "may come back" but that won't happen anymore.

Now, WHY it was removed I don't really remember. It has been 5 years since. I vaguely recall it not being compatible with all the new Python async stuff -- better leave it to the application code to choose in what way it wants or needs to implement asynchronicity, than enforcing a method in the library that doesn't integrate.

pevogam commented 2 days ago

The upgrading from Pyro4 does mention it though:

* async proxy removed (may come back but probably not directly integrated into the Proxy class)

Exactly but as I mentioned above it does explain what this change was done, i.e. the motivation behind it.

It says "may come back" but that won't happen anymore.

Now, WHY it was removed I don't really remember. It has been 5 years since. I vaguely recall it not being compatible with all the new Python async stuff -- better leave it to the application code to choose in what way it wants or needs to implement asynchronicity, than enforcing a method in the library that doesn't integrate.

At least I have run Pyro4 on some of the most recent Python 3 versions and it worked out of the box. If the application code wants to choose its own way to implement asynchronicity, then can't it simply not call this method? In other words, I don't see where the enforcing stems from. Perhaps the key answer in your sentences is "doesn't integrate" - I guess there is no way to have an async application code that is compatible with regular pyro use even if it doesn't call this method in any way?

irmen commented 2 days ago

I think the problem was that Pyro4 enforced the use of background threads to achieve the "async" calls. And that the choice to do that and how to do that is better left to the application code. Pyro has never been able to do true async calls (where they are integrated into an async event loop kinda thing).