Skip to content

Commit

Permalink
Merge pull request #620 from Avaiga/feature/#411-raise-error-if-confi…
Browse files Browse the repository at this point in the history
…g-id-is-entity-attribute

Feature/#411 - Raise error if config_id is an entity attribute
  • Loading branch information
trgiangdo authored Dec 21, 2023
2 parents 79dbae2 + 97522d8 commit 4d40830
Show file tree
Hide file tree
Showing 15 changed files with 186 additions and 23 deletions.
34 changes: 33 additions & 1 deletion taipy/core/config/checkers/_data_node_config_checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,15 @@
# specific language governing permissions and limitations under the License.

from datetime import timedelta
from typing import Dict
from typing import Dict, List

from taipy.config._config import _Config
from taipy.config.checker._checker import _ConfigChecker
from taipy.config.checker.issue_collector import IssueCollector
from taipy.config.common.scope import Scope

from ...scenario.scenario import Scenario
from ...task.task import Task
from ..data_node_config import DataNodeConfig


Expand All @@ -26,9 +28,17 @@ def __init__(self, config: _Config, collector: IssueCollector):

def _check(self) -> IssueCollector:
data_node_configs: Dict[str, DataNodeConfig] = self._config._sections[DataNodeConfig.name]
task_attributes = [attr for attr in dir(Task) if not callable(getattr(Task, attr)) and not attr.startswith("_")]
scenario_attributes = [
attr for attr in dir(Scenario) if not callable(getattr(Scenario, attr)) and not attr.startswith("_")
]

for data_node_config_id, data_node_config in data_node_configs.items():
self._check_existing_config_id(data_node_config)
self._check_if_entity_property_key_used_is_predefined(data_node_config)
self._check_if_config_id_is_overlapping_with_task_and_scenario_attributes(
data_node_config_id, data_node_config, task_attributes, scenario_attributes
)
self._check_storage_type(data_node_config_id, data_node_config)
self._check_scope(data_node_config_id, data_node_config)
self._check_validity_period(data_node_config_id, data_node_config)
Expand All @@ -38,6 +48,28 @@ def _check(self) -> IssueCollector:
self._check_exposed_type(data_node_config_id, data_node_config)
return self._collector

def _check_if_config_id_is_overlapping_with_task_and_scenario_attributes(
self,
data_node_config_id: str,
data_node_config: DataNodeConfig,
task_attributes: List[str],
scenario_attributes: List[str],
):
if data_node_config.id in task_attributes:
self._error(
data_node_config._ID_KEY,
data_node_config.id,
f"The id of the DataNodeConfig `{data_node_config_id}` is overlapping with the "
f"attribute `{data_node_config.id}` of a Task entity.",
)
elif data_node_config.id in scenario_attributes:
self._error(
data_node_config._ID_KEY,
data_node_config.id,
f"The id of the DataNodeConfig `{data_node_config_id}` is overlapping with the "
f"attribute `{data_node_config.id}` of a Scenario entity.",
)

def _check_storage_type(self, data_node_config_id: str, data_node_config: DataNodeConfig):
if data_node_config.storage_type not in DataNodeConfig._ALL_STORAGE_TYPES:
self._error(
Expand Down
21 changes: 21 additions & 0 deletions taipy/core/config/checkers/_task_config_checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,13 @@
# an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the
# specific language governing permissions and limitations under the License.

from typing import List

from taipy.config._config import _Config
from taipy.config.checker._checkers._config_checker import _ConfigChecker
from taipy.config.checker.issue_collector import IssueCollector

from ...scenario.scenario import Scenario
from ..data_node_config import DataNodeConfig
from ..task_config import TaskConfig

Expand All @@ -23,15 +26,33 @@ def __init__(self, config: _Config, collector: IssueCollector):

def _check(self) -> IssueCollector:
task_configs = self._config._sections[TaskConfig.name]
scenario_attributes = [
attr for attr in dir(Scenario) if not callable(getattr(Scenario, attr)) and not attr.startswith("_")
]

for task_config_id, task_config in task_configs.items():
if task_config_id != _Config.DEFAULT_KEY:
self._check_existing_config_id(task_config)
self._check_if_entity_property_key_used_is_predefined(task_config)
self._check_if_config_id_is_overlapping_with_scenario_attributes(
task_config_id, task_config, scenario_attributes
)
self._check_existing_function(task_config_id, task_config)
self._check_inputs(task_config_id, task_config)
self._check_outputs(task_config_id, task_config)
return self._collector

def _check_if_config_id_is_overlapping_with_scenario_attributes(
self, task_config_id: str, task_config: TaskConfig, scenario_attributes: List[str]
):
if task_config.id in scenario_attributes:
self._error(
task_config._ID_KEY,
task_config.id,
f"The id of the TaskConfig `{task_config_id}` is overlapping with the "
f"attribute `{task_config.id}` of a Scenario entity.",
)

def _check_inputs(self, task_config_id: str, task_config: TaskConfig):
self._check_children(
TaskConfig, task_config_id, task_config._INPUT_KEY, task_config.input_configs, DataNodeConfig
Expand Down
13 changes: 11 additions & 2 deletions taipy/core/data/data_node.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
from typing import Any, Dict, List, Optional, Set, Tuple, Union

import networkx as nx

from taipy.config.common._validate_id import _validate_id
from taipy.config.common.scope import Scope
from taipy.logger._taipy_logger import _TaipyLogger
Expand Down Expand Up @@ -103,9 +104,9 @@ def __init__(
editor_expiration_date: Optional[datetime] = None,
**kwargs,
):
self.config_id = _validate_id(config_id)
self._config_id = _validate_id(config_id)
self.id = id or DataNodeId(self.__ID_SEPARATOR.join([self._ID_PREFIX, self.config_id, str(uuid.uuid4())]))
self.owner_id = owner_id
self._owner_id = owner_id
self._parent_ids = parent_ids or set()
self._scope = scope
self._last_edit_date = last_edit_date
Expand All @@ -120,6 +121,14 @@ def __init__(

self._properties = _Properties(self, **kwargs)

@property
def config_id(self):
return self._config_id

@property
def owner_id(self):
return self._owner_id

def get_parents(self):
"""Get all parents of this data node."""
from ... import core as tp
Expand Down
13 changes: 10 additions & 3 deletions taipy/core/scenario/scenario.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
from typing import Any, Callable, Dict, List, Optional, Set, Union

import networkx as nx

from taipy.config.common._template_handler import _TemplateHandler as _tpl
from taipy.config.common._validate_id import _validate_id

Expand All @@ -29,7 +30,6 @@
from ..common._listattributes import _ListAttributes
from ..common._utils import _Subscriber
from ..cycle.cycle import Cycle
from ..data._data_manager_factory import _DataManagerFactory
from ..data.data_node import DataNode
from ..data.data_node_id import DataNodeId
from ..exceptions.exceptions import (
Expand All @@ -42,7 +42,6 @@
from ..job.job import Job
from ..notification import Event, EventEntityType, EventOperation, Notifier, _make_event
from ..sequence.sequence import Sequence
from ..task._task_manager_factory import _TaskManagerFactory
from ..task.task import Task
from ..task.task_id import TaskId
from .scenario_id import ScenarioId
Expand Down Expand Up @@ -96,7 +95,7 @@ def __init__(
sequences: Optional[Dict[str, Dict]] = None,
):
super().__init__(subscribers or [])
self.config_id = _validate_id(config_id)
self._config_id = _validate_id(config_id)
self.id: ScenarioId = scenario_id or self._new_id(self.config_id)

self._tasks: Union[Set[TaskId], Set[Task], Set] = tasks or set()
Expand Down Expand Up @@ -156,6 +155,10 @@ def __getattr__(self, attribute_name):
return data_nodes[protected_attribute_name]
raise AttributeError(f"{attribute_name} is not an attribute of scenario {self.id}")

@property
def config_id(self):
return self._config_id

@property # type: ignore
@_self_reload(_MANAGER_NAME)
def sequences(self) -> Dict[str, Sequence]:
Expand Down Expand Up @@ -305,6 +308,8 @@ def tasks(self) -> Dict[str, Task]:
return self.__get_tasks()

def __get_tasks(self) -> Dict[str, Task]:
from ..task._task_manager_factory import _TaskManagerFactory

_tasks = {}
task_manager = _TaskManagerFactory._build_manager()

Expand All @@ -327,6 +332,8 @@ def additional_data_nodes(self) -> Dict[str, DataNode]:
return self.__get_additional_data_nodes()

def __get_additional_data_nodes(self):
from ..data._data_manager_factory import _DataManagerFactory

additional_data_nodes = {}
data_manager = _DataManagerFactory._build_manager()

Expand Down
6 changes: 5 additions & 1 deletion taipy/core/sequence/sequence.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ def __init__(
super().__init__(subscribers)
self.id: SequenceId = sequence_id
self._tasks = tasks
self.owner_id = owner_id
self._owner_id = owner_id
self._parent_ids = parent_ids or set()
self._properties = _Properties(self, **properties)
self._version = version or _VersionManagerFactory._build_manager()._get_latest_version()
Expand Down Expand Up @@ -118,6 +118,10 @@ def data_nodes(self) -> Dict[str, DataNode]:
def parent_ids(self):
return self._parent_ids

@property
def owner_id(self):
return self._owner_id

@property
def version(self):
return self._version
Expand Down
14 changes: 10 additions & 4 deletions taipy/core/task/task.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,7 @@
from .._entity._properties import _Properties
from .._entity._reload import _Reloader, _self_reload, _self_setter
from .._version._version_manager_factory import _VersionManagerFactory
from ..data._data_manager_factory import _DataManagerFactory
from ..data.data_node import DataNode
from ..exceptions.exceptions import NonExistingDataNode
from ..notification.event import Event, EventEntityType, EventOperation, _make_event
from .task_id import TaskId

Expand Down Expand Up @@ -71,9 +69,9 @@ def __init__(
version: Optional[str] = None,
skippable: bool = False,
):
self.config_id = _validate_id(config_id)
self._config_id = _validate_id(config_id)
self.id = id or TaskId(self.__ID_SEPARATOR.join([self._ID_PREFIX, self.config_id, str(uuid.uuid4())]))
self.owner_id = owner_id
self._owner_id = owner_id
self._parent_ids = parent_ids or set()
self.__input = {dn.config_id: dn for dn in input or []}
self.__output = {dn.config_id: dn for dn in output or []}
Expand Down Expand Up @@ -109,6 +107,14 @@ def properties(self):
self._properties = _Reloader()._reload(self._MANAGER_NAME, self)._properties
return self._properties

@property
def config_id(self):
return self._config_id

@property
def owner_id(self):
return self._owner_id

def get_parents(self):
"""Get parents of the task."""
from ... import core as tp
Expand Down
3 changes: 2 additions & 1 deletion tests/core/_orchestrator/_dispatcher/test_job_dispatcher.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
from unittest.mock import MagicMock

from pytest import raises

from taipy.config.config import Config
from taipy.core import DataNodeId, JobId, TaskId
from taipy.core._orchestrator._dispatcher._development_job_dispatcher import _DevelopmentJobDispatcher
Expand Down Expand Up @@ -146,7 +147,7 @@ def test_exception_in_writing_data():
job_id = JobId("id1")
output = MagicMock()
output.id = DataNodeId("output_id")
output.config_id = "my_raising_datanode"
output._config_id = "my_raising_datanode"
output._is_in_cache = False
output.write.side_effect = ValueError()
task = Task(config_id="name", properties={}, input=[], function=print, output=[output], id=task_id)
Expand Down
42 changes: 42 additions & 0 deletions tests/core/config/checkers/test_data_node_config_checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,48 @@ def test_check_config_id(self, caplog):
Config.check()
assert len(Config._collector.errors) == 0

def test_check_config_id_is_different_from_task_and_scenario_attributes(self, caplog):
Config._collector = IssueCollector()
config = Config._applied_config
Config._compile_configs()
Config.check()
assert len(Config._collector.errors) == 0

config._sections[DataNodeConfig.name]["new"] = copy(config._sections[DataNodeConfig.name]["default"])

for conflict_id in [
"function",
"input",
"output",
"parent_ids",
"scope",
"skippable",
"additional_data_nodes",
"config_id",
"creation_date",
"cycle",
"data_nodes",
"is_primary",
"name",
"owner_id",
"properties",
"sequences",
"subscribers",
"tags",
"tasks",
"version",
]:
config._sections[DataNodeConfig.name]["new"].id = conflict_id

with pytest.raises(SystemExit):
Config._collector = IssueCollector()
Config.check()
assert len(Config._collector.errors) == 1
expected_error_message = (
f"The id of the DataNodeConfig `new` is overlapping with the attribute `{conflict_id}` of a"
)
assert expected_error_message in caplog.text

def test_check_if_entity_property_key_used_is_predefined(self, caplog):
Config._collector = IssueCollector()
config = Config._applied_config
Expand Down
38 changes: 38 additions & 0 deletions tests/core/config/checkers/test_task_config_checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
from copy import copy

import pytest

from taipy.config.checker.issue_collector import IssueCollector
from taipy.config.config import Config
from taipy.core.config import TaskConfig
Expand Down Expand Up @@ -48,6 +49,43 @@ def test_check_config_id(self, caplog):
assert len(Config._collector.errors) == 1
assert len(Config._collector.warnings) == 2

def test_check_config_id_is_different_from_all_task_properties(self, caplog):
Config._collector = IssueCollector()
config = Config._applied_config
Config._compile_configs()
Config.check()
assert len(Config._collector.errors) == 0

config._sections[TaskConfig.name]["new"] = copy(config._sections[TaskConfig.name]["default"])

for conflict_id in [
"additional_data_nodes",
"config_id",
"creation_date",
"cycle",
"data_nodes",
"is_primary",
"name",
"owner_id",
"properties",
"sequences",
"subscribers",
"tags",
"tasks",
"version",
]:
config._sections[TaskConfig.name]["new"].id = conflict_id

with pytest.raises(SystemExit):
Config._collector = IssueCollector()
Config.check()
assert len(Config._collector.errors) == 2
expected_error_message = (
"The id of the TaskConfig `new` is overlapping with the attribute"
f" `{conflict_id}` of a Scenario entity."
)
assert expected_error_message in caplog.text

def test_check_if_entity_property_key_used_is_predefined(self, caplog):
Config._collector = IssueCollector()
config = Config._applied_config
Expand Down
Loading

0 comments on commit 4d40830

Please sign in to comment.