swiss-seismological-service / scrtdd

Double Difference Relocator for SeisComP
27 stars 10 forks source link

Do not inherit global bindings #53

Closed damb closed 3 years ago

damb commented 3 years ago

scrtdd does not make use of bindings, anyway.

luca-s commented 3 years ago

Thanks for this @damb . However, I don't remeber exactly if scrtdd uses global bindings (they give the default sensor/locid for a station). Did you check that? Do you know what are the implication of setting inherit-global-bindings to false?

damb commented 3 years ago

Actually, scrtdd does not make use of bindings configuration at all (i.e. it doesn't load any configuration from SeisComP's configuration database.) scrtdd exclusively loads its configuration from its module configuration files.

The inherit-global-bindings configuration parameter only has an impact on whether to display global bindings configuration for those modules offering module bindings configuration. Since the definition of inherit-global-bindings is optional, it could well be left undefined.

When displaying the module configuration (based on the module configuration XML files) by means of e.g. scconfig a Seiscomp::System::Model tree from Seiscomp::System::SchemaDefinitions (https://github.com/SeisComP/common/blob/master/libs/seiscomp/system/schema.h#L385) is created while, again, the SchemaDefinitions are loaded from a Seiscomp::IO::XMLArchive. Within the factory method (i.e. Seiscomp::System::Model::create) it is decided whether to include and display the global bindings or not (see https://github.com/SeisComP/common/blob/2f3d8729ebbd666e821d6e9950b86f00534fdee5/libs/seiscomp/system/model.cpp#L1930-L1932).

To conclude: As long as scrtdd does not provide bindings configurations it is save to set inherit-global-bindings to false. In case of those modules not offering bindings most probably it would be the best to not define the attribute at all. If you want me to do so, let me know.

luca-s commented 3 years ago

Thanks a lot for the detailed explanation. I'll merge this now