imbajin commented on code in PR #320:
URL: https://github.com/apache/hugegraph-ai/pull/320#discussion_r3117839142
##########
hugegraph-python-client/src/pyhugegraph/api/auth.py:
##########
@@ -23,12 +23,12 @@
class AuthManager(HugeParamsBase):
- @router.http("GET", "auth/users")
+ @router.http("GET", "/auth/users")
Review Comment:
‼️ **Auth route prefix change relies on PathFilter (will break when
PathFilter is removed)**
All auth routes are changed from relative (`"auth/users"`) to absolute
(`"/auth/users"`). In `resolve()`, a leading `/` bypasses the graph scope
entirely, producing `http://host/auth/users` instead of
`http://host/graphs/{graph}/auth/users`.
This works in HugeGraph 1.7.0 only because `PathFilter` whitelists
`"auth/users"` and passes it through. **PathFilter is a temporary compatibility
layer that will be removed.**
In 1.7.0, the server-side auth APIs have migrated to
`graphspaces/{graphspace}/auth/...` (except GroupAPI which is `/auth/groups`).
The Java Client handles this with a dual-path strategy:
```java
"graphspaces/%s/auth/%s" // graphspace-scoped (User, Access, Belong, Target)
"auth/%s" // server-level (Group)
```
Recommendation: implement a similar approach in the Python Client — auth
endpoints need their own resolve logic rather than relying on PathFilter
whitelisting.
##########
hugegraph-python-client/src/pyhugegraph/api/schema_manage/edge_label.py:
##########
@@ -95,6 +95,15 @@ def enableLabelIndex(self, flag) -> "EdgeLabel":
self._parameter_holder.set("enable_label_index", flag)
return self
+ @decorator_params
+ def parent(self, parent_label) -> "EdgeLabel":
+ """
+ Set parent edge label for supporting parent & child edge label type
(HugeGraph 1.7.0+).
+ When an edge label has a parent, it becomes a child edge label with
inherited properties.
+ """
Review Comment:
⚠️ **Missing `edgelabel_type` field — server will silently ignore
`parent_label`**
I verified the server-side `EdgeLabelAPI.JsonEdgeLabel` in HugeGraph 1.7.0.
The JSON serialization uses:
- `"parent_label"` ✅ (correct field name)
- `"edgelabel_type"` ❌ (missing from this PR)
The server requires `"edgelabel_type": "SUB"` to recognize a child edge
label. Without it, the edge label is treated as `NORMAL` and `parent_label` is
silently ignored.
Suggested fix:
```python
@decorator_params
def parent(self, parent_label) -> "EdgeLabel":
self._parameter_holder.set("parent_label", parent_label)
self._parameter_holder.set("edgelabel_type", "SUB")
return self
```
Also add `"edgelabel_type"` to the `create()` whitelist, and consider adding
an `asBase()` method for creating parent edge labels (`"edgelabel_type":
"PARENT"`).
##########
hugegraph-python-client/src/pyhugegraph/structure/gremlin_data.py:
##########
@@ -70,4 +70,11 @@ def to_json(self):
class GremlinDataEncoder(json.JSONEncoder):
def default(self, o):
- return {k.split("__")[1]: v for k, v in vars(o).items()}
+ data = {}
+ for k, v in vars(o).items():
+ # Filter out None values and empty collections
+ if v is None or (isinstance(v, (dict, list)) and not v):
Review Comment:
⚠️ **Filtering empty collections may break Gremlin requests**
This changes global serialization behavior. Some Gremlin requests may
intentionally send empty `bindings: {}` or `aliases: {}` — the server may
expect these fields to be present even when empty. Filtering them out could
cause unexpected server-side errors.
Consider only filtering `None` values, not empty collections:
```python
if v is None:
continue
```
--
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]
