Load global config if the local one does not exist #3664
Conversation
happz
added
area | config
happz
force-pushed
the
config-global-path
branch
from
d6b2463 to
2f41080
Compare
| except (fmf.utils.FileError, fmf.utils.RootError): | ||
| self.logger.debug(f"Config tree not found in user path '{self.user_path}'.") | ||
|
|
||
| try: | ||
| return (self.global_path, fmf.Tree(self.global_path)) |
There was a problem hiding this comment.
With this, IIUC,global config will only be fetched if self.user_path is Not existed, right?
Think about one scenario:
user do have some setting in user path, but not containing the implementation for boot.method,
with boot.method mr landed, we will broadcast we support boot.method for mrack, but if users put something to users path, they will find boot.method or virtualization.is_support, or anything we are gonna to support would Not work.
Did I miss something? 😅
There was a problem hiding this comment.
Yes, if a user lacks translation for boot.method, but global configuration does have one, it would not be used. But that's on purpose: the user provided their own configuration, and we should use it. Even if we think we have something better in the default one, it's not our decision.
If the user has a HW config in ~/.config/tmt/hardware.fmf, and they lackbeaker.translations key, our MrackHardware.translations should step in become an empty list, and I think there's the missing piece: MrackHardware.translations does not have default factory set, so it needs to be provided by configuration, which is not necessarily happening. I think we should add default factory to HardwareConfig and MrackHardware classes to provide sane but empty defaults if a key is not set.
we will broadcast we support boot.method for mrack, but if users put something to users path, they will find boot.method or virtualization.is_support, or anything we are gonna to support would Not work.
IIUIC, you are worried that we add some new HW transition to our global configuration, but users will miss it because they already have some custom content in their local configuration. That is a valid concern, I do not have the solution yet, but I'd argue it's a different problem for another day - related, but not to be solved by leaking the "local vs default" logic into plugins. Because we will also need some kind of "repository" config, e.g. to disable some lint checks for a given repo, so we are looking at some kind of composition of global aka default, local and in-repo configuration, so that stresses out the effort of not leaking the logic into code that is using Config - must be solved there.
I added the topic to tomorrow's meeting, let's discuss there, but I'm pretty sure I for one do not want "default config" to start appearing in plugins :)
Pull Request Checklist