[Bug] rpyc 6 breaks connectors to remote modules

[qku]

Version

Development

What is affected by the bug?

Local modules cannot connect to remote modules (e.g. local logic module to remote hardware module).

When does the bug occur?

Upon loading a module which connect to a remote module.

For me, this bug only occurs with rpyc version 6. It does not occur for rpyc 5.3.1

How do we replicate the issue?

1. Configure the server: single hardware module (I used data instream dummy), allow remote
2. Configure the client: remote hardware module, logic module with connector to remote hardware module (e.g. time series reader logic)
3. Start server and load hardware module
4. Start client and attempt to load logic module (hardware module by itself loads fine)

Expected behavior

Load the logic module without errors

Relevant log output

Traceback (most recent call last):
  File "/Users/kilian/projects/qudi/qudi-core/src/qudi/core/modulemanager.py", line 208, in activate_module
    self._modules[module_name].activate()
  File "/Users/kilian/projects/qudi/qudi-core/src/qudi/core/modulemanager.py", line 530, in activate
    module.activate()
  File "/Users/kilian/projects/qudi/qudi-core/src/qudi/core/modulemanager.py", line 533, in activate
    self._connect()
  File "/Users/kilian/projects/qudi/qudi-core/src/qudi/core/modulemanager.py", line 761, in _connect
    self._instance.connect_modules(module_connections)
  File "/Users/kilian/projects/qudi/qudi-core/src/qudi/core/module.py", line 408, in connect_modules
    conn.connect(target)
  File "/Users/kilian/projects/qudi/qudi-core/src/qudi/core/connector.py", line 90, in connect
    raise RuntimeError(
RuntimeError: Module "<qudi.hardware.dummy.data_instream_dummy.InStreamDummy(0x600001e92ba0) at 0x7fc35103ee80>" connected to connector "streamer" does not implement interface "DataInStreamInterface".

Additional Comments

No response

[qku]

I seems like the underlying issue is know for rpyc and is described here: tomerfiliba-org/rpyc#355

[qku]

Here you can clearly see the different behaviour from the IPython console:

with rpyc 5.3.1

dummy = instream_dummy

dummy.__class__
Out[2]: <class 'qudi.hardware.dummy.data_instream_dummy.InStreamDummy'>

[cls.__name__ for cls in dummy.__class__.mro()]
Out[2]: ['InStreamDummy', 'DataInStreamInterface', 'Base', 'QObject', 'Object', 'object']

type(dummy)
Out[3]: <netref class 'rpyc.core.netref.qudi.hardware.dummy.data_instream_dummy.InStreamDummy'>

dummy.__class__.mro()
Out[4]: [<class 'qudi.hardware.dummy.data_instream_dummy.InStreamDummy'>, <class 'qudi.interface.data_instream_interface.DataInStreamInterface'>, <class 'qudi.core.module.Base'>, <class 'PySide2.QtCore.QObject'>, <class 'Shiboken.Object'>, <class 'object'>]

with rpyc 6.0.1

dummy = instream_dummy

dummy.__class__
Out[2]: <class 'module'>

[cls.__name__ for cls in dummy.__class__.mro()]
Out[9]: ['module', 'object']

type(dummy)
Out[3]: <netref class 'rpyc.core.netref.qudi.hardware.dummy.data_instream_dummy.InStreamDummy'>

dummy.__class__.mro()
Out[4]: [<class 'module'>, <class 'object'>]

At first I thought that the type method yields a usable result to check for connector compatibility, but it doesn't show the interface name.

[Neverhorst]

Well, this has been kind of an issue since the introduction of rpyc in qudi.
A use case that is much better covered by rpyc is using isinstance for checking base classes. However this does not work for string name comparisons which we do right now.

The upcoming major version of qudi will require Connector meta objects to be initialized with the actual interface type instead of its name string. This behaviour is way less ambiguous and solves the problem by simply using isinstance for interface checking, which seems to work in rpyc versions 5 and 6.

As a way forward I would propose fixing rpyc < 6.0.0 for now and with the upcoming major release I would set it to rpyc >= 6.0.0. This would also make PR #120 unnecessary, which would otherwise introduce more complexity for an issue that should be solved by its root.
This would also postpone the merging of PR #112 until the major release since it is not needed until then.

What do you think about this @qku @TobiasSpohn ?

[TobiasSpohn]

I think it is a good idea to fix rpyc < 6.0.0. Then new installations will work until we ironed out all the bugs introduced by the new rpyc version in PR #112 .

[Neverhorst]

Fixed requirement rpyc>=5.0.1, <6 in current main for now. It should resolve this issue and the topic will be revisited before the next major release.

[qku]

Alight! Not ideal for me as I need a package for specific hardware that requires rpyc >= 6.0 🙈 but I understand your reasoning @Neverhorst !