From c4217b16d52c6cac0a6227cf314d7c68581c9523 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A5vard=20Berland?= Date: Thu, 9 Jan 2025 09:42:38 +0100 Subject: [PATCH] Remove exec_env This feature has been deprecated since 2019 and can finally be removed. Its last user was the RMS forward model step, which no longer uses it. --- .../forward_model_step.py | 11 ----- src/ert/config/ert_config.py | 3 -- src/ert/config/forward_model_step.py | 9 ---- .../config/parsing/forward_model_keywords.py | 1 - .../config/parsing/forward_model_schema.py | 11 ----- .../unit_tests/config/test_forward_model.py | 29 ------------ .../unit_tests/ensemble_evaluator/conftest.py | 1 - .../test_forward_model_runner.py | 47 ------------------- .../forward_model_runner/test_job_dispatch.py | 4 -- 9 files changed, 116 deletions(-) diff --git a/src/_ert/forward_model_runner/forward_model_step.py b/src/_ert/forward_model_runner/forward_model_step.py index 79c5e95a854..6a277247d59 100644 --- a/src/_ert/forward_model_runner/forward_model_step.py +++ b/src/_ert/forward_model_runner/forward_model_step.py @@ -2,7 +2,6 @@ import contextlib import io -import json import logging import os import signal @@ -103,7 +102,6 @@ def create_start_message_and_check_job_files(self) -> Start: errors = self._check_job_files() errors.extend(self._assert_arg_list()) - self._dump_exec_env() if errors: start_message = start_message.with_error("\n".join(errors)) @@ -317,15 +315,6 @@ def _handle_process_io_error_and_create_exited_message( def name(self) -> str: return self.job_data["name"] - def _dump_exec_env(self) -> None: - exec_env = self.job_data.get("exec_env") - if exec_env: - exec_name, _ = os.path.splitext( - os.path.basename(cast(Path, self.job_data.get("executable"))) - ) - with open(f"{exec_name}_exec_env.json", "w", encoding="utf-8") as f_handle: - f_handle.write(json.dumps(exec_env, indent=4)) - def _check_job_files(self) -> list[str]: """ Returns the empty list if no failed checks, or a list of errors in case diff --git a/src/ert/config/ert_config.py b/src/ert/config/ert_config.py index a1010e75961..c3081a979a5 100644 --- a/src/ert/config/ert_config.py +++ b/src/ert/config/ert_config.py @@ -193,7 +193,6 @@ def handle_default(fm_step: ForwardModelStep, arg: str) -> str: "environment": substituter.filter_env_dict( dict(env_pr_fm_step.get(fm_step.name, {}), **fm_step.environment) ), - "exec_env": substituter.filter_env_dict(fm_step.exec_env), "max_running_minutes": fm_step.max_running_minutes, } @@ -1133,7 +1132,6 @@ def _forward_model_step_from_config_file( ) environment = {k: v for [k, v] in content_dict.get("ENV", [])} - exec_env = {k: v for [k, v] in content_dict.get("EXEC_ENV", [])} default_mapping = {k: v for [k, v] in content_dict.get("DEFAULT", [])} return ForwardModelStep( @@ -1152,7 +1150,6 @@ def _forward_model_step_from_config_file( arg_types=arg_types_list, environment=environment, required_keywords=content_dict.get("REQUIRED", []), - exec_env=exec_env, default_mapping=default_mapping, ) except OSError as err: diff --git a/src/ert/config/forward_model_step.py b/src/ert/config/forward_model_step.py index 31bcbdba0a6..fd631be67c6 100644 --- a/src/ert/config/forward_model_step.py +++ b/src/ert/config/forward_model_step.py @@ -50,8 +50,6 @@ class ForwardModelStepJSON(TypedDict): argList: List of command line arguments to be given to the executable. environment: Dictionary of environment variables to inject into the environment of the forward model step run - exec_env: Dictionary of environment variables to inject into the execution - environment of the forward model step. max_running_minutes: Maximum runtime in minutes. If the forward model step takes longer than this, the job is requested to be cancelled. """ @@ -66,7 +64,6 @@ class ForwardModelStepJSON(TypedDict): stdin: str argList: list[str] environment: dict[str, str] - exec_env: dict[str, str] max_running_minutes: int @@ -79,7 +76,6 @@ class ForwardModelStepOptions(TypedDict, total=False): error_file: NotRequired[str] max_running_minutes: NotRequired[int] environment: NotRequired[dict[str, str | int]] - exec_env: NotRequired[dict[str, str | int]] default_mapping: NotRequired[dict[str, str | int]] @@ -131,8 +127,6 @@ class ForwardModelStep: and thus also subject to substitution. environment: Dictionary representing environment variables to inject into the environment of the forward model step run - exec_env: Dictionary of environment variables to inject into the execution - environment of the forward model step. default_mapping: Default values for optional arguments provided by the user. For example { "A": "default_A" } private_args: A dictionary of user-provided keyword arguments. @@ -155,7 +149,6 @@ class ForwardModelStep: required_keywords: list[str] = field(default_factory=list) arg_types: list[SchemaItemType] = field(default_factory=list) environment: dict[str, int | str] = field(default_factory=dict) - exec_env: dict[str, int | str] = field(default_factory=dict) default_mapping: dict[str, int | str] = field(default_factory=dict) private_args: Substitutions = field(default_factory=Substitutions) @@ -230,7 +223,6 @@ def __init__( error_file = kwargs.get("error_file") max_running_minutes = kwargs.get("max_running_minutes") environment = kwargs.get("environment", {}) or {} - exec_env = kwargs.get("exec_env", {}) or {} default_mapping = kwargs.get("default_mapping", {}) or {} super().__init__( @@ -249,7 +241,6 @@ def __init__( required_keywords=[], arg_types=[], environment=environment, - exec_env=exec_env, default_mapping=default_mapping, private_args=Substitutions(), ) diff --git a/src/ert/config/parsing/forward_model_keywords.py b/src/ert/config/parsing/forward_model_keywords.py index f06b10d684e..11da5782e63 100644 --- a/src/ert/config/parsing/forward_model_keywords.py +++ b/src/ert/config/parsing/forward_model_keywords.py @@ -24,6 +24,5 @@ class ForwardModelStepKeys(StrEnum): REQUIRED = "REQUIRED" ENV = "ENV" - EXEC_ENV = "EXEC_ENV" DEFAULT = "DEFAULT" PRIVATE_ARGS = "PRIVATE_ARGS" diff --git a/src/ert/config/parsing/forward_model_schema.py b/src/ert/config/parsing/forward_model_schema.py index 9f2e659719d..5837d8b3ecf 100644 --- a/src/ert/config/parsing/forward_model_schema.py +++ b/src/ert/config/parsing/forward_model_schema.py @@ -117,16 +117,6 @@ def env_keyword() -> SchemaItem: ) -def exec_env_keyword() -> SchemaItem: - return SchemaItem( - kw=ForwardModelStepKeys.EXEC_ENV, - argc_min=2, - argc_max=2, - multi_occurrence=True, - type_map=[SchemaItemType.STRING, SchemaItemType.STRING], - ) - - def default_keyword() -> SchemaItem: return SchemaItem( kw=ForwardModelStepKeys.DEFAULT, @@ -153,7 +143,6 @@ def default_keyword() -> SchemaItem: default_keyword(), # Default values for args arg_type_keyword(), env_keyword(), - exec_env_keyword(), ] forward_model_deprecations: list[DeprecationInfo] = [ diff --git a/tests/ert/unit_tests/config/test_forward_model.py b/tests/ert/unit_tests/config/test_forward_model.py index 5e007bfd4cb..45b647df78b 100644 --- a/tests/ert/unit_tests/config/test_forward_model.py +++ b/tests/ert/unit_tests/config/test_forward_model.py @@ -177,34 +177,6 @@ def test_forward_model_optionals( assert forward_model.name == "config_file" -@pytest.mark.usefixtures("use_tmpdir") -def test_forward_model_env_and_exec_env_is_set(): - with open("exec", "w", encoding="utf-8") as f: - pass - - os.chmod("exec", stat.S_IXUSR | stat.S_IXGRP | stat.S_IXOTH) - - with open("CONFIG", "w", encoding="utf-8") as f: - f.write( - dedent( - """ - EXECUTABLE exec - ENV a b - ENV c d - EXEC_ENV a1 b1 - EXEC_ENV c1 d1 - """ - ) - ) - forward_model = _forward_model_step_from_config_file("CONFIG") - - assert forward_model.environment["a"] == "b" - assert forward_model.environment["c"] == "d" - - assert forward_model.exec_env["a1"] == "b1" - assert forward_model.exec_env["c1"] == "d1" - - @pytest.mark.usefixtures("use_tmpdir") def test_forward_model_stdout_stderr_defaults_to_filename(): with open("exec", "w", encoding="utf-8") as f: @@ -706,7 +678,6 @@ def validate_pre_realization_run( "_ERT_REALIZATION_NUMBER": "", "_ERT_RUNPATH": "", }, - "exec_env": {}, "default_mapping": {}, "private_args": Substitutions( { diff --git a/tests/ert/unit_tests/ensemble_evaluator/conftest.py b/tests/ert/unit_tests/ensemble_evaluator/conftest.py index 18b7dca55b3..1c2ddb76cf4 100644 --- a/tests/ert/unit_tests/ensemble_evaluator/conftest.py +++ b/tests/ert/unit_tests/ensemble_evaluator/conftest.py @@ -158,7 +158,6 @@ def _dump_forward_model(forward_model, index): "stderr": f"{index}.stderr", "stdin": forward_model.stdin_file, "environment": None, - "exec_env": {}, "max_running_minutes": forward_model.max_running_minutes, "min_arg": forward_model.min_arg, "max_arg": forward_model.max_arg, diff --git a/tests/ert/unit_tests/forward_model_runner/test_forward_model_runner.py b/tests/ert/unit_tests/forward_model_runner/test_forward_model_runner.py index 5fc64f75e4e..757d5726cc9 100644 --- a/tests/ert/unit_tests/forward_model_runner/test_forward_model_runner.py +++ b/tests/ert/unit_tests/forward_model_runner/test_forward_model_runner.py @@ -210,53 +210,6 @@ def test_run_multiple_fail_only_runs_one(): assert status.exit_code == i + 1 -@pytest.mark.usefixtures("use_tmpdir") -def test_exec_env(): - with open("exec_env.py", "w", encoding="utf-8") as f: - f.write( - """#!/usr/bin/env python\n -import os -import json -with open("exec_env_exec_env.json") as f: - exec_env = json.load(f) -assert exec_env["TEST_ENV"] == "123" - """ - ) - os.chmod("exec_env.py", stat.S_IEXEC + stat.S_IREAD) - - with open("EXEC_ENV", "w", encoding="utf-8") as f: - f.write("EXECUTABLE exec_env.py\n") - f.write("EXEC_ENV TEST_ENV 123\n") - - forward_model = _forward_model_step_from_config_file( - name=None, config_file="EXEC_ENV" - ) - - with open("jobs.json", mode="w", encoding="utf-8") as fptr: - ert_config = ErtConfig(forward_model_steps=[forward_model]) - json.dump( - create_forward_model_json( - context=ert_config.substitutions, - forward_model_steps=ert_config.forward_model_steps, - env_vars=ert_config.env_vars, - user_config_file=ert_config.user_config_file, - run_id="run_id", - ), - fptr, - ) - - with open("jobs.json", encoding="utf-8") as f: - jobs_json = json.load(f) - - for msg in list(ForwardModelRunner(jobs_json).run([])): - if isinstance(msg, Start): - with open("exec_env_exec_env.json", encoding="utf-8") as f: - exec_env = json.load(f) - assert exec_env["TEST_ENV"] == "123" - if isinstance(msg, Exited): - assert msg.exit_code == 0 - - @pytest.mark.usefixtures("use_tmpdir") def test_env_var_available_inside_step_context(): with open("run_me.py", "w", encoding="utf-8") as f: diff --git a/tests/ert/unit_tests/forward_model_runner/test_job_dispatch.py b/tests/ert/unit_tests/forward_model_runner/test_job_dispatch.py index b7e9102ca50..b28d64b35c4 100644 --- a/tests/ert/unit_tests/forward_model_runner/test_job_dispatch.py +++ b/tests/ert/unit_tests/forward_model_runner/test_job_dispatch.py @@ -62,7 +62,6 @@ def test_terminate_steps(): "stdin": None, "argList": ["3"], "environment": None, - "exec_env": None, "license_path": None, "max_running_minutes": None, "min_arg": 1, @@ -190,7 +189,6 @@ def test_fm_dispatch_run_subset_specified_as_parameter(): "stdin": None, "argList": ["A"], "environment": None, - "exec_env": None, "license_path": None, "max_running_minutes": None, "min_arg": 1, @@ -208,7 +206,6 @@ def test_fm_dispatch_run_subset_specified_as_parameter(): "stdin": None, "argList": ["B"], "environment": None, - "exec_env": None, "license_path": None, "max_running_minutes": None, "min_arg": 1, @@ -226,7 +223,6 @@ def test_fm_dispatch_run_subset_specified_as_parameter(): "stdin": None, "argList": ["C"], "environment": None, - "exec_env": None, "license_path": None, "max_running_minutes": None, "min_arg": 1,