avoid reloading same HWOBJ for YAML files

[elmjag]

Implement same behavior for YAML configured HWOBJs as for XML configured HWOBJs.

If an HWOBJ is already loaded, reuse it.

Also changes the behavior on missing YAML config files. Now it adds an error message to the summary table, rather that aborting the load process with an exception. This is the same behavior as with missing XML files.

[rhfogh]

I think we ought to talk about this.

First, why is this PR necessary? In the XML world it was routine to load the same hardware object from lots of different places, so you wanted to protect against double loads. In the yaml world, all hardware objects should be loaded from one location only - in part to make sure that you do not end up loading what should be the same HWO from different files with different parameters. Instead of facilitating double loads, why not throw an error, or at least a warning, so we can get rid of them?

Second, it looks like you are doing a lot of work to make sure that we can store, track, and use hardware objects by the name of the configuration file that describes them. Getting rid of this was actually a primary driver for changing the xml configuration system in the first place. File names are unstandardised, site specific, trivially changeable, and using them is a prime risk of causing tricky bugs. Instead of promoting the less unsafe use of file names, why not get rid of it? In my opinion this facility should disappear, the sooner the better. Why not make a start now? Or, if this is needed as a temporary bridging fix, should that not be made very clear in all places where it is used, so it is easier to remove later?

[elmjag]

Hah, I did not understand that dealing with this issue was in scope for this branch. But, let's give it a try then.

Is the idea to have a strict tree structure of hardware objects. That is, each hardware object only allowed to have one parent object. If some other object need access, it should access to it via the canonical parent?

For example, currently session.yml is refered in many places. It's refered in beamline_config.yml:

class: mxcubecore.HardwareObjects.Beamline.Beamline
objects:
  !!omap
  # Hardware:
  - session: session.yml

It is also refered in lims.yml:

class: ISPyBClientMockup.ISPyBClientMockup
configuration:
  ws_root: "https://your.lims.org"
  ws_username: ''
objects:
  session: session.yml

In this case, we designate the Beamline object as sessions's canonical parent. We remove session: session.yml line from all YAML files except for beamline_config.yml.

We modify ISPyBClientMockup class code to access session hardware object using HWR.beamline.session expression.

Is this something like that was planned? I can try to modify mocked beamline to work according to this schema.

[rhfogh]

@elmjag
I take your point that this was not necessarily on the TODO list for this branch. We could have left that part alone. But when you start doing extra work to make sure these features work better, I do think it would be better to do the extra work to prepare to get rid of them instead - or at least make it clear in the code that this is transitory.

The plan was, as you say, that each hardware object should only be configured in a single place, to get a tree structure. With the new system we can always access the HWO we want starting from the beamline. There is no need - and considerable confusion - to configure the same object in multiple places. So, yes, it would be great if you could do the changes you outlined above, now that this part of the code needs modification anyway.

[elmjag]

I tested enforcing a strict tree structure of the hardware objects on the mocked beamline.

I removed all duplicate loads of child objects from yaml config files. To my surprise it seems to work without any changes to the involved hardware objects python code.

I checked the python code for these hardware objects, and none seems to be using these child object internally. They where all using HWR.beamline.* expressions. The mocked beamline itself seems to work fine. It's possible that something subtle did break, but I did not notice it.

So, in conclusion, I feel that start enforcing strict tree structure of hardware objects is feasible at this point. I made this PR: #1001

[elmjag]

Oh yeah, closing this PR. I guess there is no reason to go this route.