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

[NEAT-250] 🐛DMSImporter missing referenced views #452

Merged
merged 6 commits into from
May 15, 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
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.
doctrino marked this conversation as resolved.
Show resolved Hide resolved
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]:
doctrino marked this conversation as resolved.
Show resolved Hide resolved
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
Loading