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
Changes from 1 commit
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
Next Next commit
Refactor version checks
oyvindeide committed Sep 2, 2024
commit 915228093645337e3624563f22485503ee8f4926
63 changes: 34 additions & 29 deletions src/ert/storage/local_storage.py
Original file line number Diff line number Diff line change
@@ -67,6 +67,8 @@ class LocalStorage(BaseMode):
"""

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

def __init__(
self,
@@ -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:
@@ -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
@@ -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,
@@ -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 :]:
@@ -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"])
@@ -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
10 changes: 5 additions & 5 deletions tests/unit_tests/storage/test_local_storage.py
Original file line number Diff line number Diff line change
@@ -252,9 +252,9 @@ 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):
@@ -271,7 +271,7 @@ def test_that_open_storage_in_read_mode_with_newer_version_throws_exception(

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",
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")

@@ -299,7 +299,7 @@ def test_that_open_storage_in_write_mode_with_newer_version_throws_exception(

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",
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")