You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hugegraph.apache.org by "neoblackcap (via GitHub)" <gi...@apache.org> on 2026/05/11 09:03:57 UTC
[PR] fix: can't query vertex with number vertex id [hugegraph-ai]
neoblackcap opened a new pull request, #329:
URL: https://github.com/apache/hugegraph-ai/pull/329
1. make vertex api compactible with number type vertex id
2. add test case for add/append/eliminate/remove vertex with number type vertex id
--
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: issues-unsubscribe@hugegraph.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@hugegraph.apache.org
For additional commands, e-mail: issues-help@hugegraph.apache.org
Re: [PR] fix: can't query vertex with number vertex id [hugegraph-ai]
Posted by "VGalaxies (via GitHub)" <gi...@apache.org>.
VGalaxies commented on code in PR #329:
URL: https://github.com/apache/hugegraph-ai/pull/329#discussion_r3371623914
##########
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}')
Review Comment:
**Medium: Undefined logger in number ID deletion test**
`hugegraph-python-client/src/tests/api/test_graph.py:116`
**Evidence**
- `test_remove_vertex_by_number_id()` catches `NotFoundError`, then calls `logger.info(...)`, but this file only imports `unittest`, `NotFoundError`, and `ClientUtils`.
**Impact**
- When `removeVertexById()` works and `getVertexById()` raises the expected `NotFoundError`, the test fails with `NameError` instead of asserting the client behavior.
**Requested fix**
- Remove the debug logging line or import/define a logger before using it.
--
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: issues-unsubscribe@hugegraph.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@hugegraph.apache.org
For additional commands, e-mail: issues-help@hugegraph.apache.org
Re: [PR] feat(llm): support number vertexid query [hugegraph-ai]
Posted by "imbajin (via GitHub)" <gi...@apache.org>.
imbajin merged PR #329:
URL: https://github.com/apache/hugegraph-ai/pull/329
--
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: issues-unsubscribe@hugegraph.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@hugegraph.apache.org
For additional commands, e-mail: issues-help@hugegraph.apache.org
Re: [PR] fix: can't query vertex with number vertex id [hugegraph-ai]
Posted by "imbajin (via GitHub)" <gi...@apache.org>.
imbajin commented on code in PR #329:
URL: https://github.com/apache/hugegraph-ai/pull/329#discussion_r3241437418
##########
hugegraph-python-client/src/pyhugegraph/utils/huge_router.py:
##########
@@ -17,6 +17,7 @@
import functools
import inspect
+import json
Review Comment:
+1 (same suggestion)
--
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: issues-unsubscribe@hugegraph.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@hugegraph.apache.org
For additional commands, e-mail: issues-help@hugegraph.apache.org
Re: [PR] fix: can't query vertex with number vertex id [hugegraph-ai]
Posted by "imbajin (via GitHub)" <gi...@apache.org>.
imbajin commented on code in PR #329:
URL: https://github.com/apache/hugegraph-ai/pull/329#discussion_r3241581104
##########
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)
+
+ for vertex_id in quoted_vertex_ids:
+ path += f'ids={vertex_id}&' # pylint: disable=consider-using-join
path = path.rstrip("&")
Review Comment:
⚠️ No test added for `getVerticesById` with numeric IDs. This is the only changed code path in this method but none of the new test cases call it with an integer vertex ID. Please add a test (once the `department` schema ID strategy is fixed):
```python
def test_get_vertices_by_number_id(self):
vertex = self.graph.addVertex("department", {"name": "DeptA", "headcount": 10, "floor": 1})
result = self.graph.getVerticesById([vertex.id])
self.assertIsNotNone(result)
self.assertEqual(result[0].id, vertex.id)
```
--
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: issues-unsubscribe@hugegraph.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@hugegraph.apache.org
For additional commands, e-mail: issues-help@hugegraph.apache.org
Re: [PR] fix: can't query vertex with number vertex id [hugegraph-ai]
Posted by "imbajin (via GitHub)" <gi...@apache.org>.
imbajin commented on code in PR #329:
URL: https://github.com/apache/hugegraph-ai/pull/329#discussion_r3241437418
##########
hugegraph-python-client/src/pyhugegraph/utils/huge_router.py:
##########
@@ -17,6 +17,7 @@
import functools
import inspect
+import json
Review Comment:
+1 (same suggestion)
--
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: issues-unsubscribe@hugegraph.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@hugegraph.apache.org
For additional commands, e-mail: issues-help@hugegraph.apache.org
Re: [PR] fix: can't query vertex with number vertex id [hugegraph-ai]
Posted by "imbajin (via GitHub)" <gi...@apache.org>.
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: issues-unsubscribe@hugegraph.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@hugegraph.apache.org
For additional commands, e-mail: issues-help@hugegraph.apache.org
Re: [PR] fix: can't query vertex with number vertex id [hugegraph-ai]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #329:
URL: https://github.com/apache/hugegraph-ai/pull/329#discussion_r3224388932
##########
hugegraph-python-client/src/pyhugegraph/utils/huge_router.py:
##########
@@ -17,6 +17,7 @@
import functools
import inspect
+import json
Review Comment:
`import json` is currently unused in this module (the vertex_id quoting logic concatenates strings manually). Either remove this import or switch the quoting/escaping to use `json.dumps()` so the import is justified.
##########
hugegraph-python-client/src/pyhugegraph/utils/huge_router.py:
##########
@@ -148,6 +149,14 @@ def wrapper(self: "HGraphContext", *args: Any, **kwargs: Any) -> Any:
all_kwargs["graphspace"] = graphspace_arg or graphspace_cfg
formatted_path = path.format(**all_kwargs)
+ elif "{vertex_id}" in path:
+ # fix vertex_id format process
+ # only quote string type vertex_id
+ vertex_id = all_kwargs.pop("vertex_id")
+ if isinstance(vertex_id, str):
+ vertex_id = "\"" + vertex_id + "\""
+ all_kwargs['vertex_id'] = vertex_id
Review Comment:
The special-casing for "{vertex_id}" applies to any route containing that placeholder (e.g. TraverserManager routes like `vertex="{vertex_id}"`), which will now double-quote string ids and produce invalid URLs (e.g. `vertex=""person:marko""`). Restrict this logic to vertex-id path segments only (e.g. when the template contains `/{vertex_id}`) and prefer `json.dumps(vertex_id)` for correct escaping instead of manual `"` concatenation.
##########
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}')
Review Comment:
`logger` is used but never imported/defined in this test file, which will raise `NameError`. Import the project logger (or remove the log line) so the test runs reliably.
##########
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))
+
Review Comment:
This test only asserts inside the `except NotFoundError` block; if `getVertexById()` unexpectedly succeeds (or raises a different exception), the test will pass silently. Use `with self.assertRaises(NotFoundError)` (or add an `else: self.fail(...)`) to ensure the expected failure is enforced.
##########
hugegraph-python-client/src/tests/client_utils.py:
##########
@@ -68,6 +70,9 @@ def init_vertex_label(self):
schema.vertexLabel("book").useCustomizeStringId().properties("name", "price").nullableKeys(
"price"
).ifNotExist().create()
+ schema.vertexLabel("department").properties("name", "headcount", "floor").nullableKeys(
Review Comment:
The new `department` vertex label doesn’t set an id strategy or primary keys. Given other labels explicitly call `.primaryKeys(...)` or `.useCustomizeStringId()`, this risks schema creation failing (PRIMARY_KEY strategy requires primary keys) and it doesn’t guarantee numeric vertex ids for the new tests. Consider explicitly setting `.useAutomaticId()` (or `.useCustomizeNumberId()` and passing `id=` when adding vertices) to ensure numeric ids.
--
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: issues-unsubscribe@hugegraph.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@hugegraph.apache.org
For additional commands, e-mail: issues-help@hugegraph.apache.org