reframe
reframe copied to clipboard
[feat] Support multiple configuration files
This PR introduces some changes for the configuration:
- ReFrame will always start from a basic minimal configuration with a
generic
system (local scheduler and launcher) and thebuiltin
environment. It also includes the basic logging handler and handlers_perflog - The user can pass multiple configuration files and they will be added one after the other. If systems/env etc have the same name the last one will be kept.
Will close #1725 .
Regarding the usage of this feature, I was thinking the following:
- We should not change
-C
to accept multiple configuration files. - Have a
RFM_CONFIG_PATH
type of variable that reframe will look for asettings.py
orsettings.json
file in each path and load it in order. - The
-C config_file
will replace completely the configuration chain as of now unless it is written as-C +config_file
in which case it will be applied incrementally on top of the configurations from theRFM_CONFIG_PATH
. - We could also cover the case of different configuration files per system by interpreting the
--system
option and then looking for a specific configuration file (we lose the auto-selection in this case, though).
@ekouts @victorusu What do you think?
Regarding the usage of this feature, I was thinking the following:
1. We should not change`-C` to accept multiple configuration files. 2. Have a `RFM_CONFIG_PATH` type of variable that reframe will look for a `settings.py` or `settings.json` file in each path and load it in order. 3. The `-C config_file` will replace completely the configuration chain as of now unless it is written as `-C +config_file` in which case it will be applied incrementally on top of the configurations from the `RFM_CONFIG_PATH`. 4. We could also cover the case of different configuration files per system by interpreting the `--system` option and then looking for a specific configuration file (we lose the auto-selection in this case, though).
@ekouts @victorusu What do you think?
- I am fine with that, the original suggestion from the issue was
-C file1:file2:file3
so this is why I started with multiple files. - I guess we also accept full path of configurations? Or do we want only files named
settings.py
? - 👍
- I don't understand what you mean here. How we would look for the right config file?
- What happens now if the user doesn't give a config at all? We used to look by default in
${HOME}/.reframe/settings.{py,json}
,${RFM_INSTALL_PREFIX}/settings.{py,json}
,/etc/reframe.d/settings.{py,json}
. I guess we keep that the same? But then the user still needs to write all parts of his configurations, they cannot avoid thelogging
unless they know the path to thereframe/core/settings
and useRFM_CONFIG_PATH
.
Regarding the usage of this feature, I was thinking the following:
- We should not change
-C
to accept multiple configuration files.- Have a
RFM_CONFIG_PATH
type of variable that reframe will look for asettings.py
orsettings.json
file in each path and load it in order.- The
-C config_file
will replace completely the configuration chain as of now unless it is written as-C +config_file
in which case it will be applied incrementally on top of the configurations from theRFM_CONFIG_PATH
.- We could also cover the case of different configuration files per system by interpreting the
--system
option and then looking for a specific configuration file (we lose the auto-selection in this case, though).@ekouts @victorusu What do you think?
- My issue with multiple files is the consistency... We have a plethora of different ways to pass various options. For instance, the
-J
option can be passed multiple times to aggregate different job options. If we use the-C
with the proposed syntax-C file1:file2:file3
, then it becomes inconsistent. I prefer to avoid accepting multiple files in a single option. We need to review this anyway for all the other options to make everything consistent. Probably, we should move to action-based commands, as the "new" tools use in the wild, as proposed by @teojgo. - If I understand correctly, the
RFM_CONFIG_PATH
environment will accept the syntaxpath1:path2:path3
, as is the normality for BASH. Then thesettings.py
andsettings.json
can be provided in those paths. I vote for that! - I am not so sure if we should do the
+
syntax here... I would vote for consistency and use the same construction that we use for-J
, based on the arguments I gave on 1. - I agree with this. I am not sure how this will be used in practice, but this is in principle the right way of doing it.
@victorusu about (1) I think @vkarak suggests to keep -C
as it was before, it will accept only one file and the user cannot pass multiple config files from the command line, so no inconsistency there. I only mentioned the user's suggestion since the intention was to give multiple config files from the cli. I think the env var would be enough.
About (2) I assume it would make sense to have config files with the machine name instead of different directories, like config/dom.py
, config/daint.py
etc instead of config/dom/settings.py
, config/daint/settings.py
, etc.. But if you both think it's better I am fine with it.
I guess we also accept full path of configurations? Or do we want only files named settings.py?
I would say no and just use directories with settings.py
inside them, so as to be consistent with other PATH variables in Linux, as @victorusu explained.
I don't understand what you mean here. How we would look for the right config file?
That's easy; we simply look for a specific config file pattern that includes the system name, e.g., settings-daint.py
instead of settings.py
. But as @victorusu pointed out that it's not clear how we use that in practice, we can wait for this behaviour until we have a concrete example.
What happens now if the user doesn't give a config at all? We used to look by default in ${HOME}/.reframe/settings.{py,json}, ${RFM_INSTALL_PREFIX}/settings.{py,json}, /etc/reframe.d/settings.{py,json}. I guess we keep that the same? But then the user still needs to write all parts of his configurations, they cannot avoid the logging unless they know the path to the reframe/core/settings and use RFM_CONFIG_PATH.
For your first part of the question, the answer is yes. We will put those in the path. For the second, the user only needs to do -C +myconfig
. But, frankly, it might be better to always include the reframe/core/settings.py
regardless of whether the path is followed or not.
I am not so sure if we should do the + syntax here... I would vote for consistency and use the same construction that we use for -J, based on the arguments I gave on 1.
I cannot understand this very well.
I am a bit puzzled of what is the best for the default behaviour of the -C
option, because eventually, I believe that we should always build incrementally on top of reframe/core/settings.py
. So the most consistent behaviour would be for -C
to add to the configuration to those found in the path, including the builtin. We could have a syntax such as -C :myconfig.py
(or pick one) that would allow users to tell reframe to use their config file exclusively. What do you think?
I am a bit puzzled of what is the best for the default behaviour of the
-C
option, because eventually, I believe that we should always build incrementally on top ofreframe/core/settings.py
. So the most consistent behaviour would be for-C
to add to the configuration to those found in the path, including the builtin. We could have a syntax such as-C :myconfig.py
(or pick one) that would allow users to tell reframe to use their config file exclusively. What do you think?
I agree that -C
should always build incrementally. Than we should allow to pass multiple -C
s... Which means that this cmdline would be acceptable -C config1.py -C config2.json -C config3.py
Having said that, I am not sure if the -C :myconfig.py
is the right syntax. Probably yes, but I am not so sure...
When passing multiple options one would do the following...
reframe -C config1.py -C config2.json -C :config3.py
Is it ok?
I am a bit puzzled of what is the best for the default behaviour of the
-C
option, because eventually, I believe that we should always build incrementally on top ofreframe/core/settings.py
. So the most consistent behaviour would be for-C
to add to the configuration to those found in the path, including the builtin. We could have a syntax such as-C :myconfig.py
(or pick one) that would allow users to tell reframe to use their config file exclusively. What do you think?I agree that
-C
should always build incrementally. Than we should allow to pass multiple-C
s... Which means that this cmdline would be acceptable-C config1.py -C config2.json -C config3.py
Having said that, I am not sure if the
-C :myconfig.py
is the right syntax. Probably yes, but I am not so sure... When passing multiple options one would do the following...reframe -C config1.py -C config2.json -C :config3.py
Is it ok?
The person who wrote this (👆) doesn't know how to write in English... Sorry for that... The correct spelling is the following...
I am a bit puzzled of what is the best for the default behaviour of the
-C
option, because eventually, I believe that we should always build incrementally on top ofreframe/core/settings.py
. So the most consistent behaviour would be for-C
to add to the configuration to those found in the path, including the builtin. We could have a syntax such as-C :myconfig.py
(or pick one) that would allow users to tell reframe to use their config file exclusively. What do you think?
I agree that -C
should always build incrementally. Then we should allow passing multiple -C
s... This means that this command line would be acceptable -C config1.py -C config2.json -C config3.py
Having said that, I am not sure if the -C :myconfig.py
is the right syntax. Probably yes, but I am not so sure... When passing multiple options one would do the following...
reframe -C config1.py -C config2.json -C :config3.py
Is it ok? What do you think?
If we say that the -C
option can be specified multiple times, then we don't need the RFM_CONFIG_PATH
. Regarding the -C :myconfig.py
option, we need a way to break the chain, that's why I propose it. If a :config
entry appears, then this will supersede all configs previously loaded. Let me think a bit about it, cos this is another breaking change.
Codecov Report
Base: 86.37% // Head: 86.50% // Increases project coverage by +0.12%
:tada:
Coverage data is based on head (
c2ec2a2
) compared to base (53aad83
). Patch coverage: 91.17% of modified lines in pull request are covered.
Additional details and impacted files
@@ Coverage Diff @@
## master #2557 +/- ##
==========================================
+ Coverage 86.37% 86.50% +0.12%
==========================================
Files 60 60
Lines 11114 11108 -6
==========================================
+ Hits 9600 9609 +9
+ Misses 1514 1499 -15
Impacted Files | Coverage Δ | |
---|---|---|
reframe/core/settings.py | 100.00% <ø> (ø) |
|
reframe/frontend/cli.py | 73.44% <63.63%> (+1.14%) |
:arrow_up: |
reframe/core/config.py | 94.55% <92.75%> (+2.73%) |
:arrow_up: |
reframe/core/logging.py | 77.69% <94.73%> (+0.57%) |
:arrow_up: |
reframe/frontend/ci.py | 94.11% <100.00%> (+2.45%) |
:arrow_up: |
reframe/frontend/runreport.py | 94.61% <100.00%> (ø) |
|
reframe/core/launchers/mpi.py | 93.69% <0.00%> (-5.21%) |
:arrow_down: |
reframe/frontend/executors/policies.py | 92.50% <0.00%> (-0.28%) |
:arrow_down: |
reframe/core/schedulers/slurm.py | 53.33% <0.00%> (-0.15%) |
:arrow_down: |
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.
:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.
@jenkins-cscs retry all
@ekouts I will also update the tutorial to avoid the step of copying the builtin config. It's good if this PR is complete.
All open issues that we have discussed are done, but giving it a last try, I realised that although configs are nicely combined for systems and environments, the same is not the same for logging. The moment you decide to change a single thing in logging, e.g., add another logger that logs debug2 info to a file, you are obliged to rewrite the whole logging section, even the perflog_handlers
. The same would happen in the most common situation, where somebody wants to change only the perflog_handlers
; they would have to rewrite the whole logging section, which is not very nice...
@ekouts could you rerun the doc listings on Daint? I have changed substantially the way the configuration is presented and we'd need to rerun.
@jenkins-cscs retry all
I cannot approve because I am the original author, but ltgm
@vkarak looks good to me. ~If~ As soon as the CI passes let's merge :)