Skip to content

Commit b90018c

Browse files
authored
Bugfix EverestConfig simulation folder validation (#14140) (#14170)
* Bugfix EverestConfig simulation folder validation (#14140) Remove check-writable-path utils function (cherry picked from commit c5bee30) * Remove testcase that is no longer accurate
1 parent b640be6 commit b90018c

4 files changed

Lines changed: 52 additions & 48 deletions

File tree

src/everest/config/everest_config.py

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,6 @@
4646
InstallDataContext,
4747
check_for_duplicate_names,
4848
check_path_exists,
49-
check_writeable_path,
5049
unique_items,
5150
validate_forward_model_configs,
5251
)
@@ -878,16 +877,6 @@ def deprecate_wells(self) -> Self:
878877
ConfigWarning.deprecation_warn(_WELLS_DEPRECATION)
879878
return self
880879

881-
@model_validator(mode="after")
882-
def validate_that_environment_sim_folder_is_writeable(self) -> Self:
883-
environment = self.environment
884-
config_path = self.config_path
885-
if environment is None or config_path is None:
886-
return self
887-
888-
check_writeable_path(environment.simulation_folder, Path(config_path))
889-
return self
890-
891880
@field_validator("wells")
892881
@no_type_check
893882
@classmethod
@@ -934,10 +923,18 @@ def validate_objective_function_weights_for_all_or_none(cls, functions):
934923
@field_validator("config_path")
935924
@no_type_check
936925
@classmethod
937-
def validate_config_path_exists(cls, config_path):
938-
expanded_path = os.path.realpath(config_path)
939-
if not Path(expanded_path).exists():
940-
raise ValueError(f"no such file or directory {expanded_path}")
926+
def validate_config_path_exists_and_is_writeable(cls, config_path):
927+
"""
928+
`os.path.exists()` swallows all OSErrors instead returning false.
929+
`Path.exists()` only shares this behavior for `>py-3.12`.
930+
Replace with `Path.exists()` when we drop support for python 3.12.
931+
"""
932+
path = Path(config_path).resolve()
933+
# Will also return False if path is unreachable.
934+
if not os.path.exists(path): # ruff: ignore[os-path-exists]
935+
raise ValueError(f"No such file or directory {path!s}")
936+
if not os.access(path.parent, os.W_OK | os.X_OK):
937+
raise ValueError(f"'{path.parent!s}' is not writeable or executable")
941938
return config_path
942939

943940
def copy(self) -> "EverestConfig": # type: ignore

src/everest/config/validation_utils.py

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -301,24 +301,6 @@ def check_path_exists(
301301
raise ValueError(f"No such file or directory {exp_path}")
302302

303303

304-
def check_writeable_path(path_source: str, config_path: Path) -> None:
305-
# check that the lowest existing folder is writeable
306-
path = as_abs_path(path_source, str(config_path.parent))
307-
while True:
308-
if os.path.isdir(path):
309-
if os.access(path, os.W_OK | os.X_OK):
310-
break
311-
elif Path(path).is_file():
312-
raise ValueError(f"{path} is a file, cannot create folders inside it")
313-
parent = os.path.dirname(path)
314-
if parent == path: # ie, if path is root
315-
break
316-
path = parent
317-
318-
if not os.access(path, os.W_OK | os.X_OK):
319-
raise ValueError(f"User does not have write access to {path}")
320-
321-
322304
def validate_forward_model_configs(
323305
forward_model: list[str], install_jobs: list[InstallForwardModelStepConfig]
324306
) -> None:

tests/everest/entry_points/test_everest_entry.py

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -379,15 +379,6 @@ def test_that_everest_run_warns_on_nonempty_runpath(
379379
existing_runpath.cleanup()
380380

381381

382-
def test_that_everest_fails_when_runpath_is_a_file():
383-
with tempfile.NamedTemporaryFile() as existing_runpath:
384-
Path(existing_runpath.name).touch()
385-
with pytest.raises(ValueError, match="is a file"):
386-
everest_config_with_defaults(
387-
environment={"simulation_folder": existing_runpath.name},
388-
)
389-
390-
391382
@pytest.mark.parametrize(
392383
("server_queue_system", "simulator_queue_system"),
393384
[

tests/everest/test_everlint.py

Lines changed: 40 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -78,15 +78,15 @@ def test_extra_key(min_config):
7878
),
7979
(
8080
{"config_path": "does_not_exist"},
81-
"no such file or directory .*/does_not_exist",
81+
"No such file or directory",
8282
),
8383
(
8484
{
8585
"install_templates": [
8686
{"template": "does_not_exist", "output_file": "not_relevant"}
8787
]
8888
},
89-
"No such file or directory .*/does_not_exist",
89+
"No such file or directory",
9090
),
9191
(
9292
{"model": {"realizations": [-1]}},
@@ -135,10 +135,6 @@ def test_extra_key(min_config):
135135
{"forward_model": ["not_a_job"]},
136136
"unknown job not_a_job",
137137
),
138-
(
139-
{"environment": {"simulation_folder": "/usr/bin/unwriteable"}},
140-
"User does not have write access to",
141-
),
142138
(
143139
{"environment": {"output_folder": ("super long path" * 300)}},
144140
"output_folder\n.* File name too long",
@@ -168,6 +164,44 @@ def test_invalid_subconfig(extra_config, min_config, expected):
168164
EverestConfig(**min_config)
169165

170166

167+
def test_that_config_directory_without_write_access_raises_validation_error(
168+
min_config, tmp_path
169+
):
170+
tmp_path.mkdir(exist_ok=True)
171+
config_path = tmp_path / "config.yml"
172+
config_path.touch()
173+
original_mode = tmp_path.stat().st_mode
174+
tmp_path.chmod(0o555)
175+
min_config["config_path"] = str(config_path)
176+
177+
try:
178+
with pytest.raises(ValidationError, match=f"'{tmp_path}' is not writeable"):
179+
EverestConfig(**min_config)
180+
finally:
181+
tmp_path.chmod(original_mode)
182+
183+
184+
def test_that_config_directory_parent_without_execute_access_raises_validation_error(
185+
min_config, tmp_path
186+
):
187+
tmp_path.mkdir(exist_ok=True)
188+
config_path = tmp_path / "sub_dir"
189+
config_path.mkdir(exist_ok=True)
190+
config_file = config_path / "config.yml"
191+
config_file.touch()
192+
original_mode = tmp_path.stat().st_mode
193+
tmp_path.chmod(0o444)
194+
min_config["config_path"] = str(config_file)
195+
196+
try:
197+
with pytest.raises(
198+
ValidationError, match=f"No such file or directory {config_file}"
199+
):
200+
EverestConfig(**min_config)
201+
finally:
202+
tmp_path.chmod(original_mode)
203+
204+
171205
@pytest.mark.parametrize(
172206
("link", "source", "target"),
173207
[

0 commit comments

Comments
 (0)