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

feat(commands): re-use the cache more often #2282

Draft
wants to merge 2 commits into
base: dev
Choose a base branch
from
Draft
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
15 changes: 14 additions & 1 deletion antarest/study/business/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,13 @@
from antares.study.version import StudyVersion

from antarest.core.exceptions import CommandApplicationError
from antarest.core.interfaces.cache import CacheConstants
from antarest.core.jwt import DEFAULT_ADMIN_USER
from antarest.core.requests import RequestParameters
from antarest.core.serialization import AntaresBaseModel
from antarest.study.business.all_optional_meta import camel_case_model
from antarest.study.model import RawStudy, Study
from antarest.study.storage.rawstudy.model.filesystem.config.model import FileStudyTreeConfigDTO
from antarest.study.storage.rawstudy.model.filesystem.factory import FileStudy
from antarest.study.storage.storage_service import StudyStorageService
from antarest.study.storage.utils import is_managed
Expand All @@ -40,12 +42,23 @@ def execute_or_add_commands(
) -> None:
if isinstance(study, RawStudy):
executed_commands: t.MutableSequence[ICommand] = []
should_invalidate_cache = False
for command in commands:
if not command.can_update_study_config():
should_invalidate_cache = True
result = command.apply(file_study, listener)
if not result.status:
raise CommandApplicationError(result.message)
executed_commands.append(command)
storage_service.variant_study_service.invalidate_cache(study)
# if commands that can't update the cache are applied, we need to invalidate it.
# Otherwise, we can update it.
if should_invalidate_cache:
storage_service.variant_study_service.invalidate_cache(study)
else:
storage_service.raw_study_service.cache.put(
f"{CacheConstants.STUDY_FACTORY}/{file_study.config.study_id}",
FileStudyTreeConfigDTO.from_build_config(file_study.config).model_dump(mode="json"),
)
if not is_managed(study):
# In a previous version, de-normalization was performed asynchronously.
# However, this cause problems with concurrent file access,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -317,3 +317,7 @@ def _create_diff(self, other: "ICommand") -> t.List["ICommand"]:
@override
def get_inner_matrices(self) -> t.List[str]:
return []

@override
def can_update_study_config(self) -> bool:
return True
Original file line number Diff line number Diff line change
Expand Up @@ -403,6 +403,10 @@ def apply_binding_constraint(
study_data.tree.save(matrix_term, ["input", "bindingconstraints", matrix_id])
return CommandOutput(status=True)

@override
def can_update_study_config(self) -> bool:
return True


class CreateBindingConstraint(AbstractBindingConstraintCommand):
"""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ def _apply_config(self, study_data: FileStudyTreeConfig) -> t.Tuple[CommandOutpu

# Check if the cluster already exists in the area
version = study_data.version
cluster = create_thermal_config(version, name=self.cluster_name)
cluster = create_thermal_config(version, **self.parameters)
if any(cl.id == cluster.id for cl in area.thermals):
return (
CommandOutput(
Expand Down Expand Up @@ -247,3 +247,7 @@ def get_inner_matrices(self) -> t.List[str]:
assert_this(isinstance(self.modulation, str))
matrices.append(strip_matrix_protocol(self.modulation))
return matrices

@override
def can_update_study_config(self) -> bool:
return True
Original file line number Diff line number Diff line change
Expand Up @@ -166,3 +166,7 @@ def _create_diff(self, other: "ICommand") -> List["ICommand"]:
@override
def get_inner_matrices(self) -> List[str]:
return []

@override
def can_update_study_config(self) -> bool:
return True
Original file line number Diff line number Diff line change
Expand Up @@ -310,3 +310,7 @@ def _create_diff(self, other: "ICommand") -> List["ICommand"]:
@override
def get_inner_matrices(self) -> List[str]:
return super().get_inner_matrices()

@override
def can_update_study_config(self) -> bool:
return True
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ def _apply_config(self, study_data: FileStudyTreeConfig) -> t.Tuple[CommandOutpu

# Check if the cluster already exists in the area
version = study_data.version
cluster = create_renewable_config(version, name=self.cluster_name)
cluster = create_renewable_config(version, **self.parameters)
if any(cl.id == cluster.id for cl in area.renewables):
return (
CommandOutput(
Expand Down Expand Up @@ -187,3 +187,7 @@ def _create_diff(self, other: "ICommand") -> t.List["ICommand"]:
@override
def get_inner_matrices(self) -> t.List[str]:
return []

@override
def can_update_study_config(self) -> bool:
return True
Original file line number Diff line number Diff line change
Expand Up @@ -348,3 +348,7 @@ def get_inner_matrices(self) -> t.List[str]:
"""
matrices: t.List[str] = [strip_matrix_protocol(getattr(self, attr)) for attr in _MATRIX_NAMES]
return matrices

@override
def can_update_study_config(self) -> bool:
return True
Original file line number Diff line number Diff line change
Expand Up @@ -109,3 +109,7 @@ def _create_diff(self, other: "ICommand") -> t.List["ICommand"]:
@override
def get_inner_matrices(self) -> t.List[str]:
return []

@override
def can_update_study_config(self) -> bool:
return True
Original file line number Diff line number Diff line change
Expand Up @@ -181,3 +181,7 @@ def _build_matrix_path(matrix_path: Path) -> Path:
if not real_path.exists():
(matrix_path / "series.txt.link").rename(real_path)
return real_path

@override
def can_update_study_config(self) -> bool:
return True
8 changes: 8 additions & 0 deletions antarest/study/storage/variantstudy/model/command/icommand.py
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,14 @@ def get_inner_matrices(self) -> t.List[str]:
"""
raise NotImplementedError()

@abstractmethod
def can_update_study_config(self) -> bool:
"""
Returns whether the command can update the study config or not.
We then know if we need to invalidate the cache or not after the command is used.
"""
raise NotImplementedError()

def get_command_extractor(self) -> "CommandExtractor":
"""
Create a new `CommandExtractor` used to revert the command changes.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -310,3 +310,7 @@ def _create_diff(self, other: "ICommand") -> t.List["ICommand"]:
@override
def get_inner_matrices(self) -> t.List[str]:
return []

@override
def can_update_study_config(self) -> bool:
return True
Original file line number Diff line number Diff line change
Expand Up @@ -107,3 +107,7 @@ def _create_diff(self, other: "ICommand") -> List["ICommand"]:
@override
def get_inner_matrices(self) -> List[str]:
return []

@override
def can_update_study_config(self) -> bool:
return True
Original file line number Diff line number Diff line change
Expand Up @@ -223,3 +223,7 @@ def _remove_cluster_from_binding_constraints(self, study_data: FileStudy) -> Non
study_data.tree.delete(["input", "bindingconstraints", matrix_id])

study_data.tree.save(binding_constraints, url)

@override
def can_update_study_config(self) -> bool:
return True
Original file line number Diff line number Diff line change
Expand Up @@ -76,3 +76,7 @@ def _create_diff(self, other: "ICommand") -> List["ICommand"]:
@override
def get_inner_matrices(self) -> List[str]:
return []

@override
def can_update_study_config(self) -> bool:
return True
Original file line number Diff line number Diff line change
Expand Up @@ -180,3 +180,7 @@ def _create_diff(self, other: "ICommand") -> t.List["ICommand"]:
@override
def get_inner_matrices(self) -> t.List[str]:
return []

@override
def can_update_study_config(self) -> bool:
return True
Original file line number Diff line number Diff line change
Expand Up @@ -166,3 +166,7 @@ def _create_diff(self, other: "ICommand") -> t.List["ICommand"]:
@override
def get_inner_matrices(self) -> t.List[str]:
return []

@override
def can_update_study_config(self) -> bool:
return True
Original file line number Diff line number Diff line change
Expand Up @@ -171,3 +171,7 @@ def _create_diff(self, other: "ICommand") -> t.List["ICommand"]:
@override
def get_inner_matrices(self) -> t.List[str]:
return []

@override
def can_update_study_config(self) -> bool:
return True
Original file line number Diff line number Diff line change
Expand Up @@ -91,3 +91,7 @@ def _create_diff(self, other: "ICommand") -> t.List["ICommand"]:
@override
def get_inner_matrices(self) -> t.List[str]:
return []

@override
def can_update_study_config(self) -> bool:
return True
Original file line number Diff line number Diff line change
Expand Up @@ -123,3 +123,7 @@ def _create_diff(self, other: "ICommand") -> t.List["ICommand"]:
def get_inner_matrices(self) -> t.List[str]:
assert_this(isinstance(self.matrix, str))
return [strip_matrix_protocol(self.matrix)]

@override
def can_update_study_config(self) -> bool:
return True
Original file line number Diff line number Diff line change
Expand Up @@ -234,3 +234,7 @@ def match(self, other: "ICommand", equal: bool = False) -> bool:
if not equal:
return self.id == other.id
return super().match(other, equal)

@override
def can_update_study_config(self) -> bool:
return True
Original file line number Diff line number Diff line change
Expand Up @@ -85,3 +85,7 @@ def _create_diff(self, other: "ICommand") -> List["ICommand"]:
@override
def get_inner_matrices(self) -> List[str]:
return []

@override
def can_update_study_config(self) -> bool:
return True
Original file line number Diff line number Diff line change
Expand Up @@ -112,3 +112,7 @@ def _create_diff(self, other: "ICommand") -> t.List["ICommand"]:
@override
def get_inner_matrices(self) -> t.List[str]:
return []

@override
def can_update_study_config(self) -> bool:
return False
Original file line number Diff line number Diff line change
Expand Up @@ -134,3 +134,7 @@ def _create_diff(self, other: "ICommand") -> List["ICommand"]:
@override
def get_inner_matrices(self) -> List[str]:
return []

@override
def can_update_study_config(self) -> bool:
return True
15 changes: 15 additions & 0 deletions antarest/study/storage/variantstudy/model/command/update_link.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,17 @@ class UpdateLink(AbstractLinkCommand):

@override
def _apply_config(self, study_data: FileStudyTreeConfig) -> OutputTuple:
self.parameters = self.parameters or {}
# Only updates to reflect in the config are about the filter values
if "filter-synthesis" in self.parameters or "filter-year-by-year" in self.parameters:
area_from, area_to = sorted([self.area1, self.area2])
if "filter-synthesis" in self.parameters:
filters_synthesis = [step.strip() for step in self.parameters["filter-synthesis"].split(",")]
study_data.areas[area_from].links[area_to].filters_synthesis = filters_synthesis
if "filter-year-by-year" in self.parameters:
filters_year_by_year = [step.strip() for step in self.parameters["filter-year-by-year"].split(",")]
study_data.areas[area_from].links[area_to].filters_year = filters_year_by_year

return (
CommandOutput(
status=True,
Expand Down Expand Up @@ -84,3 +95,7 @@ def _create_diff(self, other: "ICommand") -> t.List["ICommand"]:
@override
def get_inner_matrices(self) -> t.List[str]:
return super().get_inner_matrices()

@override
def can_update_study_config(self) -> bool:
return True
Original file line number Diff line number Diff line change
Expand Up @@ -94,3 +94,7 @@ def _create_diff(self, other: "ICommand") -> List["ICommand"]:
@override
def get_inner_matrices(self) -> List[str]:
return []

@override
def can_update_study_config(self) -> bool:
return True
Original file line number Diff line number Diff line change
Expand Up @@ -96,3 +96,7 @@ def _create_diff(self, other: "ICommand") -> List["ICommand"]:
@override
def get_inner_matrices(self) -> List[str]:
return []

@override
def can_update_study_config(self) -> bool:
return False
Original file line number Diff line number Diff line change
Expand Up @@ -126,3 +126,7 @@ def _create_diff(self, other: "ICommand") -> t.List["ICommand"]:
@override
def get_inner_matrices(self) -> t.List[str]:
return []

@override
def can_update_study_config(self) -> bool:
return True
4 changes: 3 additions & 1 deletion tests/storage/test_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@
study_matcher,
)
from antarest.study.storage.variantstudy.business.matrix_constants_generator import GeneratorMatrixConstants
from antarest.study.storage.variantstudy.model.command.icommand import ICommand
from antarest.study.storage.variantstudy.model.command_context import CommandContext
from antarest.study.storage.variantstudy.model.dbmodel import VariantStudy
from antarest.study.storage.variantstudy.variant_study_service import VariantStudyService
Expand Down Expand Up @@ -1294,7 +1295,8 @@ def test_edit_study_with_command() -> None:
repository=Mock(),
config=Mock(),
)
command = Mock()
command = Mock(spec=ICommand)
command.can_update_study_config = Mock(return_value=False)
service._create_edit_study_command = Mock(return_value=command)
file_study = Mock()
file_study.config.study_id = study_id
Expand Down
8 changes: 4 additions & 4 deletions tests/variantstudy/model/command/test_create_cluster.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
import pytest
from pydantic import ValidationError

from antarest.study.model import STUDY_VERSION_8_8
from antarest.study.model import STUDY_VERSION_7_2, STUDY_VERSION_8_8
from antarest.study.storage.rawstudy.model.filesystem.config.model import transform_name_to_id
from antarest.study.storage.rawstudy.model.filesystem.factory import FileStudy
from antarest.study.storage.variantstudy.business.command_reverter import CommandReverter
Expand Down Expand Up @@ -120,7 +120,7 @@ def test_apply(self, empty_study: FileStudy, command_context: CommandContext):
prepro=prepro,
modulation=modulation,
command_context=command_context,
study_version=STUDY_VERSION_8_8,
study_version=STUDY_VERSION_7_2,
)

output = command.apply(empty_study)
Expand Down Expand Up @@ -150,7 +150,7 @@ def test_apply(self, empty_study: FileStudy, command_context: CommandContext):
prepro=prepro,
modulation=modulation,
command_context=command_context,
study_version=STUDY_VERSION_8_8,
study_version=STUDY_VERSION_7_2,
).apply(empty_study)
assert output.status is False
assert re.match(
Expand All @@ -166,7 +166,7 @@ def test_apply(self, empty_study: FileStudy, command_context: CommandContext):
prepro=prepro,
modulation=modulation,
command_context=command_context,
study_version=STUDY_VERSION_8_8,
study_version=STUDY_VERSION_7_2,
).apply(empty_study)
assert output.status is False
assert re.match(
Expand Down
Loading