Skip to content

Commit

Permalink
[NEAT-250] 🐛DMSImporter missing referenced views (#452)
Browse files Browse the repository at this point in the history
* fix: Missing reference bug

* refactor: handle connection reference

* build: changelog

* refactor: added warnings

* refactor; catch warnings
  • Loading branch information
doctrino committed May 15, 2024
1 parent eedfda0 commit 1cf9de1
Show file tree
Hide file tree
Showing 5 changed files with 87 additions and 6 deletions.
14 changes: 9 additions & 5 deletions cognite/neat/rules/importers/_dms2rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,10 @@ def __init__(
self.ref_metadata = ref_metadata
self.issue_list = IssueList(read_issues)
self._all_containers_by_id = schema.containers.copy()
self._all_view_ids = set(self.root_schema.views.keys())
if self.root_schema.reference:
self._all_containers_by_id.update(self.root_schema.reference.containers)
self._all_view_ids.update(self.root_schema.reference.views.keys())

@classmethod
def from_data_model_id(
Expand Down Expand Up @@ -100,15 +102,17 @@ def from_data_model_id(
else:
ref_model = None

try:
issue_list = IssueList()
with _handle_issues(issue_list) as result:
schema = DMSSchema.from_data_model(client, user_model, ref_model)
except Exception as e:
return cls(DMSSchema(), [issues.importing.APIError(str(e))])

if result.result == "failure" or issue_list.has_errors:
return cls(DMSSchema(), issue_list)

metadata = cls._create_metadata_from_model(user_model)
ref_metadata = cls._create_metadata_from_model(ref_model) if ref_model else None

return cls(schema, [], metadata, ref_metadata)
return cls(schema, issue_list, metadata, ref_metadata)

@classmethod
def _find_model_in_list(
Expand Down Expand Up @@ -361,7 +365,7 @@ def _get_value_type(
elif isinstance(prop, dm.MappedPropertyApply):
container_prop = self._container_prop_unsafe(cast(dm.MappedPropertyApply, prop))
if isinstance(container_prop.type, dm.DirectRelation):
if prop.source is None:
if prop.source is None or prop.source not in self._all_view_ids:
# The warning is issued when the DMS Rules are created.
return DMSUnknownEntity()
else:
Expand Down
19 changes: 19 additions & 0 deletions cognite/neat/rules/issues/dms.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
"HasDataFilterAppliedToTooManyContainersWarning",
"ReverseRelationMissingOtherSideWarning",
"NodeTypeFilterOnParentViewWarning",
"MissingViewInModelWarning",
"ChangingContainerError",
"ChangingViewError",
]
Expand Down Expand Up @@ -293,6 +294,24 @@ def dump(self) -> dict[str, Any]:
return output


@dataclass(frozen=True)
class MissingViewInModelWarning(DMSSchemaWarning):
description = "The data model contains view pointing to views not present in the data model"
fix = "Add the view(s) to the data model"
error_name: ClassVar[str] = "MissingViewInModel"
data_model_id: dm.DataModelId
view_ids: set[dm.ViewId]

def message(self) -> str:
return f"The view(s) {self.view_ids} are missing in the data model {self.data_model_id}"

def dump(self) -> dict[str, Any]:
output = super().dump()
output["data_model_id"] = self.data_model_id.dump()
output["view_id"] = [view_id.dump() for view_id in self.view_ids]
return output


@dataclass(frozen=True)
class ContainerPropertyUsedMultipleTimesError(DMSSchemaError):
description = "The container property is used multiple times by the same view property"
Expand Down
16 changes: 16 additions & 0 deletions cognite/neat/rules/issues/importing.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from abc import ABC
from dataclasses import dataclass
from typing import Any

from .base import NeatValidationError, ValidationWarning

Expand All @@ -23,6 +24,7 @@
"MissingIdentifierError",
"UnsupportedPropertyTypeError",
"APIError",
"FailedImportWarning",
]


Expand Down Expand Up @@ -259,6 +261,20 @@ def dump(self) -> dict[str, str]:
return {"error_message": self.error_message}


@dataclass(frozen=True)
class FailedImportWarning(ModelImportWarning):
description = "Failed to import part of the model."
fix = "No fix is available."

identifier: set[str]

def message(self) -> str:
return f"Failed to import: {self.identifier}. This will be skipped."

def dump(self) -> dict[str, Any]:
return {"identifier": list(self.identifier)}


@dataclass(frozen=True)
class ModelImportError(NeatValidationError, ABC):
description = "An error was raised during importing."
Expand Down
37 changes: 36 additions & 1 deletion cognite/neat/rules/models/dms/_schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
from cognite.client import data_modeling as dm
from cognite.client.data_classes import DatabaseWrite, DatabaseWriteList, TransformationWrite, TransformationWriteList
from cognite.client.data_classes.data_modeling import ViewApply
from cognite.client.data_classes.data_modeling.views import ReverseDirectRelation
from cognite.client.data_classes.transformations.common import Edges, EdgeType, Nodes, ViewInfo

from cognite.neat.rules import issues
Expand All @@ -28,6 +29,7 @@
MissingSourceViewError,
MissingSpaceError,
MissingViewError,
MissingViewInModelWarning,
)
from cognite.neat.rules.models.data_types import _DATA_TYPE_BY_DMS_TYPE
from cognite.neat.utils.cdf_classes import (
Expand Down Expand Up @@ -146,8 +148,26 @@ def from_data_model(
space_write = space_read.as_write()

view_loader = ViewLoader(client)
# We need to include parent views in the schema to make sure that the schema is valid.

existing_view_ids = set(views.as_ids())

# We need to include all views the edges/direct relations are pointing to have a complete schema.
connection_referenced_view_ids: set[dm.ViewId] = set()
for view in views:
connection_referenced_view_ids |= cls._connection_references(view)
connection_referenced_view_ids = connection_referenced_view_ids - existing_view_ids
if connection_referenced_view_ids:
warnings.warn(
MissingViewInModelWarning(data_model.as_id(), connection_referenced_view_ids), UserWarning, stacklevel=2
)
connection_referenced_views = view_loader.retrieve(list(connection_referenced_view_ids))
if failed := connection_referenced_view_ids - set(connection_referenced_views.as_ids()):
warnings.warn(
issues.importing.FailedImportWarning({repr(v) for v in failed}), UserWarning, stacklevel=2
)
views.extend(connection_referenced_views)

# We need to include parent views in the schema to make sure that the schema is valid.
parent_view_ids = {parent for view in views for parent in view.implements or []}
parents = view_loader.retrieve_all_parents(list(parent_view_ids - existing_view_ids))
views.extend([parent for parent in parents if parent.as_id() not in existing_view_ids])
Expand Down Expand Up @@ -203,6 +223,21 @@ def from_data_model(
reference=ref_schema,
)

@classmethod
def _connection_references(cls, view: dm.View) -> set[dm.ViewId]:
view_ids: set[dm.ViewId] = set()
for prop in (view.properties or {}).values():
if isinstance(prop, dm.MappedProperty) and isinstance(prop.type, dm.DirectRelation):
if prop.source:
view_ids.add(prop.source)
elif isinstance(prop, dm.EdgeConnection):
view_ids.add(prop.source)
if prop.edge_source:
view_ids.add(prop.edge_source)
elif isinstance(prop, ReverseDirectRelation):
view_ids.add(prop.source)
return view_ids

@classmethod
def from_directory(cls, directory: str | Path) -> Self:
"""Load a schema from a directory containing YAML files.
Expand Down
7 changes: 7 additions & 0 deletions docs/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,13 @@ Changes are grouped as follows:
- `Fixed` for any bug fixes.
- `Security` in case of vulnerabilities.



## TBD
### Fixed
- When using `DMSExporter` and importing a data model with a view pointing to a view not in the data model,
it would fail to convert to an `Information` rules. This is now fixed.

## [0.77.2] - 14-05-24
### Added
- Missing warning when `RawFilter` is used to warn users that the usage of this filter is not recommended.
Expand Down

0 comments on commit 1cf9de1

Please sign in to comment.