search

avocado-framework / aexpect

Python library used to control interactive programs
GNU General Public License v2.0
8 stars 32 forks source link

Provide remote Pyro5 compatibility for multiple remote objects #138

Closed pevogam closed 1 day ago

pevogam commented 1 week ago

This is a more rarely used case that we could cover with Pyro5 support as well. The advertised method remains in retrieving multiple remote objects one by one using single remote object getter and this one remains more manual and requiring of control file implementation.

pevogam commented 1 week ago

@ldoktor I hope you don't mind a small follow up, some corner cases had to be covered too even if they are not recommend and mostly backwards compatible.

pevogam commented 1 week ago

Thank you, @pevogam, for keeping this up to date. The change looks fine but TBH I don't much understand the commit message?

So far the commit message focuses on just the corner case we cover with the Pyro5 support we started earlier as this is the focus of the commit.

IIUC the Pyro5.nameserver resp. Pyro4.naming are daemons that is used by the pyro package to lookup shared functions/methods?

Yes, you get it pretty much. These are name servers that are used for resolving names of remote objects (for easier access than long hashes which are the unique default identifiers for these objects).

And IIUC the pyro4 has some limitations when working with pyro5 so you are adding a compat code to use 4 with 4 and 5 if available?

In this case we will use the correct name server in cases we have Pyro4 installed locally and Pyro4 installed remotely and resp. Pyro5 installed locally and Pyro5 installed remotely. The previous case would make something like Pyro4/Pyro5 locally work with Pyro4 remotely. Note that the current usage is more standard (expecting the same version) and is not a regression in functionality since many of the Pyro5 versions already have incompatible protocol with the latest (but no longer worked on) Pyro4 versions.

Regarding the compat mode, the reason we went for this was to introduce the minimal amount of changes to the various remote door functionality until everything is better tested over the longer term. "Tread lightly" so to say.

ldoktor commented 6 days ago

Thanks for the confirmation, would you please modify the commit message to describe what you just mentioned?...:

Why have I made these changes?
What effect have my changes made?
Why was the change needed?
What are the changes in reference to?
pevogam commented 3 days ago

Sure! Done and thanks for asking for clarifications.