Skip to content

Commit

Permalink
Prevent empty environment variable as LSF_SERVER value
Browse files Browse the repository at this point in the history
  • Loading branch information
larsevj committed Sep 22, 2023
1 parent 1f00934 commit d7d37e4
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 11 deletions.
18 changes: 9 additions & 9 deletions src/ert/config/parsing/config_schema_item.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import os
import shutil
import warnings
from typing import List, Mapping, Optional, TypeVar, Union

from pydantic import BaseModel, NonNegativeInt, PositiveInt

from .config_errors import ConfigValidationError
from .config_errors import ConfigValidationError, ConfigWarning
from .context_values import (
ContextBool,
ContextFloat,
Expand Down Expand Up @@ -87,8 +88,10 @@ def token_to_value_with_context(

if not self._is_in_allowed_values_for_arg_at_index(token, index):
raise ConfigValidationError.with_context(
f"{self.kw!r} argument {index + 1!r} must be one of"
f" {self.indexed_selection_set[index]!r} was {token.value!r}",
(
f"{self.kw!r} argument {index + 1!r} must be one of"
f" {self.indexed_selection_set[index]!r} was {token.value!r}"
),
token,
)

Expand All @@ -104,17 +107,15 @@ def token_to_value_with_context(
return ContextBool(False, token, keyword)
else:
raise ConfigValidationError.with_context(
f"{self.kw!r} must have a boolean value"
f" as argument {index + 1!r}",
f"{self.kw!r} must have a boolean value as argument {index + 1!r}",
token,
)
if val_type == SchemaItemType.INT:
try:
return ContextInt(int(token), token, keyword)
except ValueError as e:
raise ConfigValidationError.with_context(
f"{self.kw!r} must have an integer value"
f" as argument {index + 1!r}",
f"{self.kw!r} must have an integer value as argument {index + 1!r}",
token,
) from e
if val_type == SchemaItemType.FLOAT:
Expand Down Expand Up @@ -161,8 +162,7 @@ def token_to_value_with_context(

if os.path.isdir(absolute_path):
raise ConfigValidationError.with_context(
f"Expected executable file, "
f"but {token.value!r} is a directory.",
f"Expected executable file, but {token.value!r} is a directory.",
token,
)

Expand Down
11 changes: 10 additions & 1 deletion src/ert/config/queue_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,16 @@ def from_dict(cls, config_dict: ConfigDict) -> QueueConfig:
f"Valid choices are {sorted(VALID_QUEUE_OPTIONS[queue_system])}."
)
if values:
queue_options[queue_system].append((option_name, values[0]))
if option_name == "LSF_SERVER" and values[0].startswith("$"):
raise ConfigValidationError(
"Invalid server name for QUEUE_OPTION LSF LSF_SERVER:"
f" {values[0]}. Check for empty environment variable. The"
" LSF_SERVER value is usually provided by default in a"
" site-configuration file. If you have no special needs you"
" can safely remove it from your configuration."
)
else:
queue_options[queue_system].append((option_name, values[0]))
else:
queue_options[queue_system].append(option_name)

Expand Down
28 changes: 27 additions & 1 deletion tests/unit_tests/config/test_queue_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,13 @@
import pytest
from hypothesis import given

from ert.config import ConfigValidationError, ErtConfig, QueueConfig, QueueSystem
from ert.config import (
ConfigValidationError,
ConfigWarning,
ErtConfig,
QueueConfig,
QueueSystem,
)
from ert.job_queue import Driver


Expand Down Expand Up @@ -160,3 +166,23 @@ def test_overwriting_QUEUE_OPTIONS_warning(
f"Overwriting QUEUE_OPTION {queue_system} {queue_system_option}: \n Old value:"
" test_0 \n New value: test_1" in caplog.text
)


@pytest.mark.usefixtures("use_tmpdir", "set_site_config")
def test_empty_LSF_SERVER_variable():
filename = "config.ert"
with open(filename, "w", encoding="utf-8") as f:
f.write("NUM_REALIZATIONS 1\n")
f.write("QUEUE_SYSTEM LSF\n")
f.write("QUEUE_OPTION LSF LSF_SERVER $MY_SERVER\n")
with pytest.raises(
ConfigValidationError,
match=(
r"Invalid server name for QUEUE_OPTION LSF LSF_SERVER: "
r"\$MY_SERVER. Check for empty environment variable. "
r"The LSF_SERVER value is usually provided by default in a"
r" site-configuration file. If you have no special needs you"
r" can safely remove it from your configuration."
),
):
ErtConfig.from_file(filename)

0 comments on commit d7d37e4

Please sign in to comment.