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

Improve registration robustness for instances of registered classes #85

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mawildoer
Copy link

I am trying to remotely control a very OO interface by leveraging Pyro5's autoproxying features

I implemented a version of this workaround in a util function in the module I was working on and thought the same case might cause issues for others too.

The issue arrises when you do something like:

from kigadgets.util import register_return
from Pyro5.api import expose, Daemon

daemon = Daemon()

@expose
class Returned(object):
    pass  # not important

daemon.register(Returned)

@expose
class Dummy(object):
    @property
    def thingo(self) -> Returned:
        result = Returned()
        self._pyroDaemon.register(result)
        return result

dummy = Dummy()
daemon.register(dummy)
dummy.thingo

@irmen
Copy link
Owner

irmen commented Feb 1, 2024

what exactly was the issue you encountered initially?

@mawildoer
Copy link
Author

If you try register an instance of a class that's already registered, it'll raise a DaemonError.

Seems to be because the _pyroId is inherited from the parent class, and is therefore already present, even if the instance itself is unregistered.

I don't want to blindly use force=True in this case because I'm connecting up a third-party lib that is going to return other instances of exposed classes.

@irmen
Copy link
Owner

irmen commented Feb 4, 2024

Could you perhaps add a unit test for this change as well?
I think test_daemon.py could be the right place for it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants