Knowledge graph query: graph query endpoints #2941
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2941 +/- ##
==========================================
- Coverage 84.01% 83.94% -0.07%
==========================================
Files 487 494 +7
Lines 35972 36210 +238
Branches 10966 10966
==========================================
+ Hits 30222 30398 +176
- Misses 5224 5285 +61
- Partials 526 527 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
nucliadb/src/nucliadb/search/search/query_parser/parsers/graph.py
Outdated
Show resolved
Hide resolved
| def build_graph_response(results: list[nodereader_pb2.GraphSearchResponse]) -> GraphSearchResponse: | ||
| paths = [] | ||
| for shard_results in results: | ||
| for pb_path in shard_results.graph: |
There was a problem hiding this comment.
This may return duplicated path if the same path is present in multiple shards. This also means that top_k is not correctly used with multiple shards. We need something a bit more involved to merge shard results.
It will be much more relevant for searching nodes/entities. This has to be addressed at some point, not necessarily in this PR.
180ad41 to
09ed4d9
Compare
| @@ -54,6 +54,10 @@ spec: | |||
| regex: '^/api/v\d+/kb/[^/]+/suggest' | |||
| method: | |||
| regex: "GET|OPTIONS" | |||
| - uri: | |||
| regex: '^/api/v\d+/kb/[^/]+/graph' | |||
There was a problem hiding this comment.
I'll add /graph/nodes and /graph/relations when they are more ready
| class GraphNode(BaseModel): | ||
| value: Optional[str] = None | ||
| match: NodeMatchKind = NodeMatchKind.EXACT | ||
| type: Optional[RelationNodeType] = RelationNodeType.ENTITY |
There was a problem hiding this comment.
if you set a default value that is not none, you can remove the Optional and assume will always be set
There was a problem hiding this comment.
I wanted to provide the usual default but allow users to specify type=None, i.e., any type. I can remove the default if that's confusing
b9ccc9a to
d836bf3
Compare
Description
Describe the proposed changes made in this PR.
How was this PR tested?
Describe how you tested this PR.