Skip to content

Commit ab561f7

Browse files
committed
Bugfix EverestConfig simulation folder validation (#14140)
Remove check-writable-path utils function (cherry picked from commit c5bee30)
1 parent e1e2130 commit ab561f7

4 files changed

Lines changed: 55 additions & 44 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: 43 additions & 2 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]}},
@@ -168,6 +168,47 @@ def test_invalid_subconfig(extra_config, min_config, expected):
168168
EverestConfig(**min_config)
169169

170170

171+
<<<<<<< HEAD
172+
=======
173+
def test_that_config_directory_without_write_access_raises_validation_error(
174+
min_config, tmp_path
175+
):
176+
tmp_path.mkdir(exist_ok=True)
177+
config_path = tmp_path / "config.yml"
178+
config_path.touch()
179+
original_mode = tmp_path.stat().st_mode
180+
tmp_path.chmod(0o555)
181+
min_config["config_path"] = str(config_path)
182+
183+
try:
184+
with pytest.raises(ValidationError, match=f"'{tmp_path}' is not writeable"):
185+
EverestConfig(**min_config)
186+
finally:
187+
tmp_path.chmod(original_mode)
188+
189+
190+
def test_that_config_directory_parent_without_execute_access_raises_validation_error(
191+
min_config, tmp_path
192+
):
193+
tmp_path.mkdir(exist_ok=True)
194+
config_path = tmp_path / "sub_dir"
195+
config_path.mkdir(exist_ok=True)
196+
config_file = config_path / "config.yml"
197+
config_file.touch()
198+
original_mode = tmp_path.stat().st_mode
199+
tmp_path.chmod(0o444)
200+
min_config["config_path"] = str(config_file)
201+
202+
try:
203+
with pytest.raises(
204+
ValidationError, match=f"No such file or directory {config_file}"
205+
):
206+
EverestConfig(**min_config)
207+
finally:
208+
tmp_path.chmod(original_mode)
209+
210+
211+
>>>>>>> c5bee30c5e (Bugfix EverestConfig simulation folder validation (#14140))
171212
@pytest.mark.parametrize(
172213
("link", "source", "target"),
173214
[

0 commit comments

Comments
 (0)