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

feature/mx-1702 improve errors and logs #189

Merged
merged 7 commits into from
Nov 8, 2024
Merged
Show file tree
Hide file tree
Changes from 3 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
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Changes

- upgrade mex-common and mex-model dependencies to metadata model v3
- apply additional linters in prep for `all` ruff linters
- mute warnings about labels used in queries but missing in graph
- split up search_merged_items_in_graph for better readability
- update cypher queries to use `CALL` clauses with correct variable scope

### Deprecated

### Removed
Expand Down
7 changes: 5 additions & 2 deletions mex/backend/graph/connector.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
from string import Template
from typing import Annotated, Any, Literal, cast

from neo4j import Driver, GraphDatabase, NotificationMinimumSeverity
from neo4j import Driver, GraphDatabase, NotificationDisabledCategory
from pydantic import Field

from mex.backend.fields import (
Expand Down Expand Up @@ -84,7 +84,10 @@ def _init_driver(self) -> Driver:
settings.graph_password.get_secret_value(),
),
database=settings.graph_db,
warn_notification_severity=NotificationMinimumSeverity.OFF,
notifications_disabled_categories=[
# mute warnings about labels used in queries but missing in graph
NotificationDisabledCategory.UNRECOGNIZED,
],
)

def _check_connectivity_and_authentication(self) -> Result:
Expand Down
9 changes: 4 additions & 5 deletions mex/backend/graph/cypher/fetch_extracted_or_rule_items.cql
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ Returns:
contains the values of nested objects as well as the identifiers of
referenced items
-#>
CALL {
CALL () {
<%- block match_clause -%>
<%- if filter_by_query_string %>
CALL db.index.fulltext.queryNodes("search_index", $query_string)
Expand All @@ -38,18 +38,17 @@ CALL {
<%- endblock %>
RETURN COUNT(n) AS total
}
CALL {
CALL () {
<<-self.match_clause()>>
CALL {
WITH n
WITH n
CALL (n) {
OPTIONAL MATCH (n)-[r]->(referenced:<<merged_labels|join("|")>>)
RETURN CASE WHEN referenced IS NOT NULL THEN {
label: type(r),
position: r.position,
value: referenced.identifier
} ELSE NULL END as ref
UNION
WITH n
OPTIONAL MATCH (n)-[r]->(nested:<<nested_labels|join("|")>>)
RETURN CASE WHEN nested IS NOT NULL THEN {
label: type(r),
Expand Down
9 changes: 4 additions & 5 deletions mex/backend/graph/cypher/fetch_merged_items.cql
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ Returns:
merged item. Each component has an extra attribute `_refs` that contains the
values of nested objects as well as the identifiers of referenced items.
-#>
CALL {
CALL () {
<%- block match_clause -%>
<%- if filter_by_query_string %>
CALL db.index.fulltext.queryNodes("search_index", $query_string)
Expand All @@ -39,19 +39,18 @@ CALL {
<%- endblock %>
RETURN COUNT(merged) AS total
}
CALL {
CALL () {
<<-self.match_clause()>>
OPTIONAL MATCH (n)-[:stableTargetId]->(merged)
CALL {
WITH n
WITH n, merged
CALL (n) {
OPTIONAL MATCH (n)-[r]->(referenced:<<merged_labels|join("|")>>)
RETURN CASE WHEN referenced IS NOT NULL THEN {
label: type(r),
position: r.position,
value: referenced.identifier
} ELSE NULL END as ref
UNION
WITH n
OPTIONAL MATCH (n)-[r]->(nested:<<nested_labels|join("|")>>)
RETURN CASE WHEN nested IS NOT NULL THEN {
label: type(r),
Expand Down
6 changes: 3 additions & 3 deletions mex/backend/graph/cypher/merge_edges.cql
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ Returns:
pruned: Number of pruned edges
edges: List of the merged edge objects
-#>
MATCH (source:<<current_label>> {<<current_constraints|render_constraints>>})-[stableTargetId:stableTargetId]->({identifier: $stable_target_id})
CALL {
MATCH (source:<<current_label>> {<<current_constraints|render_constraints>>})-[:stableTargetId]->({identifier: $stable_target_id})
CALL (source) {
<%- if ref_labels %>
<%- set union = joiner("UNION\n ") %>
<%- for ref_label in ref_labels %>
Expand All @@ -35,7 +35,7 @@ CALL {
<%- endif %>
}
WITH source, count(edge) as merged, collect(edge) as edges
CALL {
CALL (source, edges) {
WITH source, edges
rababerladuseladim marked this conversation as resolved.
Show resolved Hide resolved
MATCH (source)-[outdated_edge]->(:<<merged_labels|join("|")>>)
WHERE NOT outdated_edge IN edges
Expand Down
2 changes: 1 addition & 1 deletion mex/backend/graph/cypher/merge_item.cql
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ ON MATCH SET value_<<index>> += $nested_values[<<index>>]
WITH current,
[<<range(nested_edge_labels|count)|map("ensure_prefix", "edge_")|join(", ")>>] as edges,
[<<range(nested_edge_labels|count)|map("ensure_prefix", "value_")|join(", ")>>] as values
CALL {
CALL (current, values) {
WITH current, values
rababerladuseladim marked this conversation as resolved.
Show resolved Hide resolved
MATCH (current)-[]->(outdated_node:<<nested_labels|join("|")>>)
WHERE NOT outdated_node IN values
Expand Down
82 changes: 50 additions & 32 deletions mex/backend/merged/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,10 @@
from mex.common.transform import ensure_prefix
from mex.common.types import Identifier

EXTRACTED_MODEL_ADAPTER: TypeAdapter[AnyExtractedModel] = TypeAdapter(
Annotated[AnyExtractedModel, Field(discriminator="entityType")]
)


def _merge_extracted_items_and_apply_preventive_rule(
merged_dict: dict[str, Any],
Expand Down Expand Up @@ -132,6 +136,43 @@ def create_merged_item(
return cast(AnyMergedModel, merged_item) # mypy, get a grip!


def merge_search_result_item(item: dict[str, Any]) -> AnyMergedModel:
"""Merge a single search result into a merged item.

Args:
item: Raw merged search result item from the graph response

Raises:
InconsistentGraphError: When the graph response item has inconsistencies

Returns:
AnyMergedModel instance
"""
extracted_items = [
EXTRACTED_MODEL_ADAPTER.validate_python(component)
for component in item["components"]
if component["entityType"] in EXTRACTED_MODEL_CLASSES_BY_NAME
]
rules_raw = [
component
for component in item["components"]
if component["entityType"] in RULE_MODEL_CLASSES_BY_NAME
]
if len(rules_raw) == NUMBER_OF_RULE_TYPES:
rule_set_response = transform_raw_rules_to_rule_set_response(rules_raw)
elif len(rules_raw) == 0:
rule_set_response = None
else:
msg = f"Unexpected number of rules found in graph: {len(rules_raw)}"
raise InconsistentGraphError(msg)

return create_merged_item(
identifier=item["identifier"],
extracted_items=extracted_items,
rule_set=rule_set_response,
)


def search_merged_items_in_graph(
query_string: str | None = None,
stable_target_id: str | None = None,
Expand All @@ -149,7 +190,7 @@ def search_merged_items_in_graph(
limit: How many items to return at most

Raises:
InconsistentGraphError: When the graph response cannot be parsed
InconsistentGraphError: When the graph response has inconsistencies

Returns:
MergedItemSearch instance
Expand All @@ -162,39 +203,16 @@ def search_merged_items_in_graph(
skip=skip,
limit=limit,
)
extracted_model_adapter: TypeAdapter[AnyExtractedModel] = TypeAdapter(
Annotated[AnyExtractedModel, Field(discriminator="entityType")]
)
items: list[AnyMergedModel] = []
total: int = result["total"]

for item in result["items"]:
extracted_items = [
extracted_model_adapter.validate_python(component)
for component in item["components"]
if component["entityType"] in EXTRACTED_MODEL_CLASSES_BY_NAME
]

rules_raw = [
component
for component in item["components"]
if component["entityType"] in RULE_MODEL_CLASSES_BY_NAME
]
if len(rules_raw) == NUMBER_OF_RULE_TYPES:
rule_set_response = transform_raw_rules_to_rule_set_response(rules_raw)
elif len(rules_raw) == 0:
rule_set_response = None
else:
msg = f"Unexpected number of rules found in graph: {len(rules_raw)}"
raise MExError(msg)

items.append(
create_merged_item(
identifier=item["identifier"],
extracted_items=extracted_items,
rule_set=rule_set_response,
)
items: list[AnyMergedModel] = [
reraising(
ValidationError,
InconsistentGraphError,
merge_search_result_item,
item,
)
for item in result["items"]
]
return reraising(
ValidationError,
InconsistentGraphError,
Expand Down
2 changes: 1 addition & 1 deletion pdm.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,10 @@ dependencies = [
"httpx>=0.27.2,<1",
"jinja2>=3.1.4,<4",
"mex-common @ git+https://github.com/robert-koch-institut/mex-common.git@0.40.0",
"neo4j>=5.24.0,<6",
"neo4j>=5.25.0,<6",
"pydantic>=2.9.1,<3",
"starlette>=0.41.2,<1",
"uvicorn[standard]>=0.30.6,<1",
"uvicorn[standard]>=0.32.0,<1",
]
optional-dependencies.dev = [
"black>=24.8.0,<25",
Expand Down
Loading