Skip to content

Commit

Permalink
Have ForwardModelRunner validate executable_file earlier
Browse files Browse the repository at this point in the history
This commit fixes the issue where the ForwardModelRunner on the job_runner side would crash when validating the existence of a ForwardModelStep executable; causing the drivers to hang while waiting for a never coming return code from JobRunner.
  • Loading branch information
jonathan-eq committed Sep 19, 2024
1 parent c2dbbd8 commit f365fef
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 16 deletions.
8 changes: 4 additions & 4 deletions src/_ert/forward_model_runner/io/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ def cond_unlink(file):
os.unlink(file)


def assert_file_executable(fname):
def check_executable(fname) -> str:
"""The function raises an IOError if the given file is either not a file or
not an executable.
Expand All @@ -17,7 +17,7 @@ def assert_file_executable(fname):
"""
if not fname:
raise IOError("No executable provided!")
return "No executable provided!"
fname = os.path.expanduser(fname)

potential_executables = [os.path.abspath(fname)]
Expand All @@ -28,7 +28,7 @@ def assert_file_executable(fname):
]

if not any(map(os.path.isfile, potential_executables)):
raise IOError(f"{fname} is not a file!")
return f"{fname} is not a file!"

if not any(os.access(fn, os.X_OK) for fn in potential_executables):
raise IOError(f"{fname} is not an executable!")
return f"{fname} is not an executable!"
10 changes: 5 additions & 5 deletions src/_ert/forward_model_runner/job.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

from psutil import AccessDenied, NoSuchProcess, Process, TimeoutExpired, ZombieProcess

from .io import assert_file_executable
from .io import check_executable
from .reporting.message import (
Exited,
MemoryStatus,
Expand Down Expand Up @@ -97,10 +97,7 @@ def run(self):

yield start_message

executable = self.job_data.get("executable")
assert_file_executable(executable)

arg_list = [executable]
arg_list = [self.job_data.get("executable")]
if self.job_data.get("argList"):
arg_list += self.job_data["argList"]

Expand Down Expand Up @@ -313,6 +310,9 @@ def _check_job_files(self):
):
os.unlink(self.job_data.get("error_file"))

if executable_error := check_executable(self.job_data.get("executable")):
errors.append(str(executable_error))

return errors

def _check_target_file_is_written(self, target_file_mtime: int, timeout=5):
Expand Down
13 changes: 6 additions & 7 deletions tests/unit_tests/forward_model_runner/test_job.py
Original file line number Diff line number Diff line change
Expand Up @@ -219,9 +219,9 @@ def test_run_with_defined_executable_but_missing():
0,
)

with pytest.raises(IOError):
for _ in job.run():
pass
start_message = next(job.run())
assert isinstance(start_message, Start)
assert "this/is/not/a/file is not a file" in start_message.error_message


@pytest.mark.usefixtures("use_tmpdir")
Expand Down Expand Up @@ -265,10 +265,9 @@ def test_run_with_defined_executable_no_exec_bit():
},
0,
)

with pytest.raises(IOError):
for _ in job.run():
pass
start_message = next(job.run())
assert isinstance(start_message, Start)
assert "foo is not an executable" in start_message.error_message


def test_init_job_no_std():
Expand Down

0 comments on commit f365fef

Please sign in to comment.