Propagate SETENV to forward model validation #9479
Conversation
berland
force-pushed
the
fix_custom_ecl_site_config
branch
2 times, most recently
from
8e4030e to
46a8419
Compare
berland
added
the
release-notes:bug-fix
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #9479 +/- ##
==========================================
+ Coverage 91.87% 91.90% +0.02%
==========================================
Files 433 433
Lines 26752 26753 +1
==========================================
+ Hits 24579 24587 +8
+ Misses 2173 2166 -7
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
The settings implied through SETENV can affect how forward model behave, and may also change how validation should be performed. This is in particular true for the Eclipse forward model steps which try to validate the requested version. If a different configuration of Eclipse is set through SETENV ECL100_SITE_CONFIG somefile.yml (or for ECL300_SITE_CONFIG) this information must be conveyed to the validation code.
berland
force-pushed
the
fix_custom_ecl_site_config
branch
from
46a8419 to
16bab37
Compare
|
Do we want to add this complexity given that we now have #9050 and that ecl config should come through that system instead? 🤔 |
| @@ -278,13 +281,17 @@ def __init__(self) -> None: | |||
| default_mapping={"<NUM_CPU>": 1, "<OPTS>": "", "<VERSION>": "version"}, | |||
| ) | |||
|
|
|||
| def validate_pre_experiment(self, _: ForwardModelStepJSON) -> None: | |||
| def validate_pre_experiment( | |||
| self, _: ForwardModelStepJSON, env_vars: Dict[str, str] | |||
There was a problem hiding this comment.
It's a bit unfortunate to break the signature of this function as it is public and used by plugins. I think it would be better to make env_vars part of ForwardModelStepJSON and be a non-required field. That way we do not break any plugins.
There was a problem hiding this comment.
Absolutely agree, this has already been done in #9108
|
Closing this, will not spend time shaping this up for a backport unless needed. Underlying problem will be fixed in #9108. |
The settings implied through SETENV can affect how forward model behave, and may also change how validation should be performed.
This is in particular true for the Eclipse forward model steps which try to validate the requested version. If a different configuration of Eclipse is set through
SETENV ECL100_SITE_CONFIG somefile.yml
(or for ECL300_SITE_CONFIG) this information must be conveyed to the validation code.
Issue
Resolves #9474
git rebase -i main --exec 'pytest tests/ert/unit_tests -n logical -m "not integration_test"')When applicable