imbajin commented on code in PR #329:
URL: https://github.com/apache/hugegraph-ai/pull/329#discussion_r3241828198
##########
hugegraph-python-client/src/pyhugegraph/api/graph.py:
##########
@@ -200,8 +200,10 @@ def getVerticesById(self, vertex_ids) -> list[VertexData]
| None:
if not vertex_ids:
return []
path = "traversers/vertices?"
- for vertex_id in vertex_ids:
- path += f'ids="{vertex_id}"&' # pylint:
disable=consider-using-join
+ quoted_vertex_ids = map(json.dumps, vertex_ids)
+
Review Comment:
⚠️ `json.dumps` without `ensure_ascii=False` will Unicode-escape non-ASCII
vertex IDs — `json.dumps("\u4eba\u540d")` produces `'"\\u4eba\\u540d"'` instead
of `'"\u4eba\u540d"'`, causing HugeGraph lookups to fail for any non-ASCII
string vertex ID.
```suggestion
quoted_vertex_ids = (json.dumps(vid, ensure_ascii=False) for vid in
vertex_ids)
```
##########
hugegraph-python-client/pyproject.toml:
##########
@@ -32,7 +32,7 @@ dependencies = [
"requests",
"setuptools",
"urllib3",
- "rich",
+ "rich"
]
Review Comment:
🧹 Removing the trailing comma after `"rich"` is unrelated to this bug fix
and adds diff noise. TOML allows trailing commas; the original style is
consistent with the rest of the list. Please revert this hunk.
##########
hugegraph-python-client/src/tests/api/test_graph.py:
##########
@@ -89,6 +106,16 @@ def test_remove_vertex_by_id(self):
except NotFoundError as e:
self.assertTrue("Alice\\' does not exist" in str(e))
+ def test_remove_vertex_by_number_id(self):
+ vertex = self.graph.addVertex("department", {"name": "DepartmentA",
"headcount": 10, "floor": 1})
+ self.graph.removeVertexById(vertex.id)
+ try:
+ self.graph.getVertexById(vertex.id)
+ except NotFoundError as e:
+ msg = "\\'{}\\' does not exist".format(vertex.id)
+ logger.info(f'test_msg: {msg}')
+ self.assertTrue(msg in str(e))
+
def test_add_edge(self):
vertex1 = self.graph.addVertex("person", {"name": "Alice", "age": 20})
Review Comment:
⚠️ The expected error pattern `"\\'{}\\' does not exist"` was copied from
the string-ID test where IDs have the form `label:name`. For AUTOMATIC numeric
IDs (e.g. `42`), HugeGraph's server error format may differ — numeric IDs may
not be wrapped in single-quotes in the error message.
Please verify the actual server error text for a numeric vertex ID and
update accordingly. Alternatively, drop the message assertion and rely only on
`assertRaises(NotFoundError)` (see also the silent-pass issue already flagged
in this block).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
