Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve error message if something goes wrong when loading storage #8606

Merged
merged 2 commits into from
Sep 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 15 additions & 4 deletions src/ert/gui/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
from qtpy.QtGui import QIcon
from qtpy.QtWidgets import QApplication, QWidget

from ert.config import ConfigValidationError, ConfigWarning, ErtConfig
from ert.config import ConfigValidationError, ConfigWarning, ErrorInfo, ErtConfig
from ert.gui.main_window import ErtMainWindow
from ert.gui.simulation import ExperimentPanel
from ert.gui.tools.event_viewer import (
Expand All @@ -37,7 +37,7 @@
from ert.namespace import Namespace
from ert.plugins import ErtPluginManager
from ert.services import StorageService
from ert.storage import Storage, open_storage
from ert.storage import ErtStorageException, Storage, open_storage
from ert.storage.local_storage import local_storage_set_ert_config

from .suggestor import Suggestor
Expand Down Expand Up @@ -89,6 +89,7 @@ def _start_initial_gui_window(
logger = logging.getLogger(__name__)
error_messages = []
config_warnings = []
deprecations = []
ert_config = None

with warnings.catch_warnings(record=True) as all_warnings:
Expand Down Expand Up @@ -116,7 +117,17 @@ def _start_initial_gui_window(
and cast(ConfigWarning, w.message).info.is_deprecation
]
error_messages += error.errors
logger.info("Error in config file shown in gui: '%s'", str(error))
if ert_config is not None:
try:
storage = open_storage(ert_config.ens_path, mode="w")
except ErtStorageException as err:
error_messages.append(
ErrorInfo(f"Error opening storage in ENSPATH: {err}").set_context(
ert_config.ens_path
)
)
if error_messages:
logger.info(f"Error in config file shown in gui: {error_messages}")
return (
Suggestor(
error_messages,
Expand All @@ -131,6 +142,7 @@ def _start_initial_gui_window(
),
None,
)
assert ert_config is not None
config_warnings = [
cast(ConfigWarning, w.message).info
for w in all_warnings
Expand Down Expand Up @@ -159,7 +171,6 @@ def _start_initial_gui_window(
logger.info("Suggestion shown in gui '%s'", msg)
for msg in config_warnings:
logger.info("Warning shown in gui '%s'", msg)
storage = open_storage(ert_config.ens_path, mode="w")
_main_window = _setup_main_window(
ert_config, args, log_handler, storage, plugin_manager
)
Expand Down
11 changes: 10 additions & 1 deletion src/ert/storage/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,19 @@
EnsembleAccessor = LocalEnsemble


class ErtStorageException(Exception):
pass


def open_storage(
path: Union[str, os.PathLike[str]], mode: Union[ModeLiteral, Mode] = "r"
) -> Storage:
return LocalStorage(Path(path), Mode(mode))
try:
return LocalStorage(Path(path), Mode(mode))
except Exception as err:
raise ErtStorageException(
f"Failed to open storage: {path} with error: {err}"
) from err


__all__ = [
Expand Down
63 changes: 34 additions & 29 deletions src/ert/storage/local_storage.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,8 @@ class LocalStorage(BaseMode):
"""

LOCK_TIMEOUT = 5
EXPERIMENTS_PATH = "experiments"
ENSEMBLES_PATH = "ensembles"

def __init__(
self,
Expand Down Expand Up @@ -95,26 +97,36 @@ def __init__(
self._ensembles: Dict[UUID, LocalEnsemble]
self._index: _Index

try:
version = _storage_version(self.path)
except FileNotFoundError as err:
# No index json, will have a problem if other components of storage exists
errors = []
if (self.path / self.EXPERIMENTS_PATH).exists():
errors.append(
f"experiments path: {(self.path / self.EXPERIMENTS_PATH)}"
)
if (self.path / self.ENSEMBLES_PATH).exists():
errors.append(f"ensemble path: {self.path / self.ENSEMBLES_PATH}")
if errors:
raise ValueError(f"No index.json, but found: {errors}") from err
version = _LOCAL_STORAGE_VERSION

if version > _LOCAL_STORAGE_VERSION:
raise RuntimeError(
f"Cannot open storage '{self.path}': Storage version {version} is newer than the current version {_LOCAL_STORAGE_VERSION}, upgrade ert to continue, or run with a different ENSPATH"
)
if self.can_write:
if not any(self.path.glob("*")):
# No point migrating if storage is empty
ignore_migration_check = True
self._acquire_lock()
if not ignore_migration_check:
self._migrate()
if version < _LOCAL_STORAGE_VERSION and not ignore_migration_check:
self._migrate(version)
self._index = self._load_index()
self._ensure_fs_version_exists()
self._save_index()
elif (version := _storage_version(self.path)) is not None:
if version < _LOCAL_STORAGE_VERSION:
raise RuntimeError(
f"Cannot open storage '{self.path}' in read-only mode: Storage version {version} is too old. Run ert to initiate migration."
)
if version > _LOCAL_STORAGE_VERSION:
raise RuntimeError(
f"Cannot open storage '{self.path}' in read-only mode: Storage version {version} is newer than the current version {_LOCAL_STORAGE_VERSION}, upgrade ert to continue, or run with a different ENSPATH"
)

elif version < _LOCAL_STORAGE_VERSION:
raise RuntimeError(
f"Cannot open storage '{self.path}' in read-only mode: Storage version {version} is too old. Run ert to initiate migration."
)
self.refresh()

def refresh(self) -> None:
Expand Down Expand Up @@ -249,10 +261,10 @@ def _load_experiments(self) -> Dict[UUID, LocalExperiment]:
}

def _ensemble_path(self, ensemble_id: UUID) -> Path:
return self.path / "ensembles" / str(ensemble_id)
return self.path / self.ENSEMBLES_PATH / str(ensemble_id)

def _experiment_path(self, experiment_id: UUID) -> Path:
return self.path / "experiments" / str(experiment_id)
return self.path / self.EXPERIMENTS_PATH / str(experiment_id)

def __enter__(self) -> LocalStorage:
return self
Expand Down Expand Up @@ -456,7 +468,7 @@ def _save_index(self) -> None:
print(self._index.model_dump_json(indent=4), file=f)

@require_write
def _migrate(self) -> None:
def _migrate(self, version: int) -> None:
from ert.storage.migration import ( # noqa: PLC0415
block_fs,
to2,
Expand All @@ -467,18 +479,12 @@ def _migrate(self) -> None:
)

try:
version = _storage_version(self.path)
assert isinstance(version, int)
self._index = self._load_index()
if version == 0:
self._release_lock()
block_fs.migrate(self.path)
self._acquire_lock()
self._add_migration_information(0, _LOCAL_STORAGE_VERSION, "block_fs")
elif version > _LOCAL_STORAGE_VERSION:
raise RuntimeError(
f"Cannot migrate storage '{self.path}'. Storage version {version} is newer than the current version {_LOCAL_STORAGE_VERSION}, upgrade ert to continue, or run with a different ENSPATH"
)
elif version < _LOCAL_STORAGE_VERSION:
migrations = list(enumerate([to2, to3, to4, to5, to6], start=1))
for from_version, migration in migrations[version - 1 :]:
Expand Down Expand Up @@ -526,10 +532,9 @@ def get_unique_experiment_name(self, experiment_name: str) -> str:
return experiment_name + "_0"


def _storage_version(path: Path) -> Optional[int]:
def _storage_version(path: Path) -> int:
if not path.exists():
logger.warning(f"Unknown storage version in '{path}'")
return None
return _LOCAL_STORAGE_VERSION
try:
with open(path / "index.json", encoding="utf-8") as f:
return int(json.load(f)["version"])
Expand All @@ -538,8 +543,8 @@ def _storage_version(path: Path) -> Optional[int]:
except FileNotFoundError:
if _is_block_storage(path):
return 0
logger.warning(f"Unknown storage version in '{path}'")
return None
else:
raise


_migration_ert_config: Optional[ErtConfig] = None
Expand Down
29 changes: 19 additions & 10 deletions tests/unit_tests/storage/test_local_storage.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import json
import os
import shutil
import tempfile
from dataclasses import dataclass, field
Expand Down Expand Up @@ -32,7 +33,7 @@
from ert.config.gen_kw_config import TransformFunctionDefinition
from ert.config.general_observation import GenObservation
from ert.config.observation_vector import ObsVector
from ert.storage import open_storage
from ert.storage import ErtStorageException, open_storage
from ert.storage.local_storage import _LOCAL_STORAGE_VERSION
from ert.storage.mode import ModeError
from ert.storage.realization_storage_state import RealizationStorageState
Expand Down Expand Up @@ -252,16 +253,24 @@ def test_open_storage_write_with_empty_directory(tmp_path, caplog):


def test_open_storage_read_with_empty_directory(tmp_path, caplog):
with open_storage(tmp_path / "storage", mode="r"):
assert len(caplog.messages) == 1
assert "Unknown storage version in" in caplog.messages[0]
with open_storage(tmp_path / "storage", mode="r") as storage:
assert list(storage.ensembles) == []
assert list(storage.experiments) == []


def test_open_storage_nested_dirs(tmp_path, caplog):
with open_storage(tmp_path / "extra_level" / "storage", mode="w") as storage:
assert storage.path.exists()


def test_open_storage_with_corrupted_storage(tmp_path):
with open_storage(tmp_path / "storage", mode="w") as storage:
storage.create_experiment().create_ensemble(name="prior", ensemble_size=1)
os.remove(tmp_path / "storage" / "index.json")
with pytest.raises(ErtStorageException, match="No index.json"):
open_storage(tmp_path / "storage", mode="w")


def test_that_open_storage_in_read_mode_with_newer_version_throws_exception(
tmp_path, caplog
):
Expand All @@ -270,8 +279,8 @@ def test_that_open_storage_in_read_mode_with_newer_version_throws_exception(
storage._save_index()

with pytest.raises(
RuntimeError,
match=f"Cannot open storage '{tmp_path}' in read-only mode: Storage version {_LOCAL_STORAGE_VERSION+1} is newer than the current version {_LOCAL_STORAGE_VERSION}, upgrade ert to continue, or run with a different ENSPATH",
ErtStorageException,
match=f"Cannot open storage '{tmp_path}': Storage version {_LOCAL_STORAGE_VERSION+1} is newer than the current version {_LOCAL_STORAGE_VERSION}, upgrade ert to continue, or run with a different ENSPATH",
):
open_storage(tmp_path, mode="r")

Expand All @@ -284,7 +293,7 @@ def test_that_open_storage_in_read_mode_with_older_version_throws_exception(
storage._save_index()

with pytest.raises(
RuntimeError,
ErtStorageException,
match=f"Cannot open storage '{tmp_path}' in read-only mode: Storage version {_LOCAL_STORAGE_VERSION-1} is too old",
):
open_storage(tmp_path, mode="r")
Expand All @@ -298,8 +307,8 @@ def test_that_open_storage_in_write_mode_with_newer_version_throws_exception(
storage._save_index()

with pytest.raises(
RuntimeError,
match=f"Cannot migrate storage '{tmp_path}'. Storage version {_LOCAL_STORAGE_VERSION+1} is newer than the current version {_LOCAL_STORAGE_VERSION}, upgrade ert to continue, or run with a different ENSPATH",
ErtStorageException,
match=f"Cannot open storage '{tmp_path}': Storage version {_LOCAL_STORAGE_VERSION+1} is newer than the current version {_LOCAL_STORAGE_VERSION}, upgrade ert to continue, or run with a different ENSPATH",
):
open_storage(tmp_path, mode="w")

Expand Down Expand Up @@ -534,7 +543,7 @@ def double_open_timeout(self):
# already opened with mode="w" somewhere else
with patch(
"ert.storage.local_storage.LocalStorage.LOCK_TIMEOUT", 0.0
), pytest.raises(TimeoutError):
), pytest.raises(ErtStorageException):
open_storage(self.tmpdir + "/storage/", mode="w")

@rule()
Expand Down