refactor: replace the IP + Port with URL#209
refactor: replace the IP + Port with URL#209imbajin merged 8 commits intoapache:mainfrom mingfang:main
Conversation
dosubot
bot
added
the
size:L
There was a problem hiding this comment.
Copilot reviewed 15 out of 15 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (1)
hugegraph-llm/src/hugegraph_llm/api/rag_api.py:142
- The function call 'apply_graph_conf' appears inconsistent with the similarly defined function 'apply_graph_config' (as seen in the configs_block file). Consider unifying the function name to avoid potential confusion.
res = apply_graph_conf(req.url, req.name, req.user, req.pwd, req.gs, origin_call="http")
| gr.Textbox(value=huge_settings.graph_ip, label="ip"), | ||
| gr.Textbox(value=huge_settings.graph_port, label="port"), |
There was a problem hiding this comment.
index error here? (start the app.py)
/hugegraph_llm/demo/rag_demo/app.py", line 140, in init_rag_ui
textbox_array_graph_config[5],
IndexError: list index out of range
Update: I fixed it in 2da1a78
There was a problem hiding this comment.
Also we need run the process to check other function (ensure working well)
There was a problem hiding this comment.
Another issue:
It seems unclear and unfriendly to report an error directly when the user does not carry the url scheme/prefix
Update: fixed in 89c4465
imbajin
changed the title
| try: | ||
| response = requests.get( | ||
| f"http://{self.ip}:{self.port}/versions", timeout=0.5 | ||
| f"{self.url}/versions", timeout=0.5 |
There was a problem hiding this comment.
Perhaps we should add a compatibility check/adapter here.
If str does not start with http/https, it will automatically concatenate http:// to avoid making modifications at all original calls. 🤔 (It also avoids the risk of errors caused by omitted modifications)
Already fixed it
There was a problem hiding this comment.
Pull Request Overview
This pull request refactors multiple modules to replace separate IP and port parameters with a unified URL parameter, addressing issue #191. Key changes include updating configuration classes, client constructors, and example/test files to use "url" consistently, as well as modifying related API endpoints and configuration docs.
Reviewed Changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| hugegraph-python-client/src/pyhugegraph/utils/huge_config.py | Replaces ip/port with url and adds compatibility check for URL prefix |
| hugegraph-python-client/src/pyhugegraph/example/hugegraph_test.py | Updates client initialization to use url instead of ip and port |
| hugegraph-python-client/src/pyhugegraph/client.py | Updates PyHugeClient constructor to pass url to HGraphConfig |
| hugegraph-ml/* | Systematically updates functions to receive url and pass to PyHugeClient |
| hugegraph-llm/* | Modifies API endpoints, config modules and examples to adopt url replacement |
| hugegraph-llm/README.md | Minor documentation update regarding poetry version |
This pull request refactors multiple modules to replace separate IP and port parameters with a unified URL parameter, addressing issue (fix #191). Key changes include updating configuration classes, client constructors, and example/test files to use "url" consistently, as well as modifying related API endpoints and configuration docs.