Muawiya-contact commented on PR #320:
URL: https://github.com/apache/hugegraph-ai/pull/320#issuecomment-4293434635

   # Question about Auth API Dual-Path Strategy Implementation
   
   Hi @imbajin,
   
   Thank you for the detailed review feedback on the Auth API refactoring. I've 
successfully implemented 3 out of 4 critical items:
   
   ✅ **Edge Label `edgelabel_type` Field** - Automatically set to "SUB" in 
`parent()` method and added to `create()` whitelist
   ✅ **GremlinDataEncoder** - Now filters only None values, preserving empty 
collections as server may expect them
   ✅ **Test Skip Logic** - Gracefully handles connection timeouts and 404 errors
   
   ---
   
   ## Challenge with Auth API Dual-Path Strategy
   
   However, I encountered a blocker implementing the dual-path strategy for 
auth endpoints. 
   
   **The Issue:**
   The current router framework uses `@router.http("METHOD", "path")` 
decorators that are evaluated at class definition time, not at runtime. This 
makes it impossible to dynamically construct paths like:
   - `graphspaces/{graphspace}/auth/users` (HugeGraph 1.7.0+)
   - `auth/users` (HugeGraph 1.x)
   
   **Attempts:**
   1. **Commit dbcbb46** - Removed decorators and added `_get_auth_path()` 
helper method for runtime path construction → ✗ All CI tests failed
   2. **Reverting to 79d1583** - Restored decorator-based approach → ✅ All CI 
tests pass, but still relies on PathFilter
   
   **Current State (Commit 6bf97c2):**
   - Auth endpoints use absolute paths: `@router.http("GET", "/auth/users")`
   - Documented PathFilter dependency in comments for future refactoring
   - All tests passing but not future-proof
   
   ---
   
   ## Questions for Guidance
   
   1. Should I pursue a deeper refactoring of the router framework to support 
dynamic path resolution?
   2. Or is the current approach acceptable as an intermediate solution with 
the PathFilter dependency documented?
   3. Are there other examples in the codebase of dynamic path construction 
that I could learn from?
   
   The PR is ready to merge with all other feedback addressed. Just want to 
ensure the Auth API approach aligns with your long-term vision for the Python 
Client.
   
   Thanks for your guidance! 🙏


-- 
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]

Reply via email to