imbajin commented on code in PR #378:
URL: https://github.com/apache/hugegraph-ai/pull/378#discussion_r4193559759
##########
hugegraph-llm/src/hugegraph_llm/demo/rag_demo/configs_block.py:
##########
@@ -779,6 +795,38 @@ def vector_engine_settings(engine):
apply_faiss_button = gr.Button("Apply Configuration")
apply_faiss_button.click(lambda: apply_vector_engine(engine))
+ def refresh_llm_config(revision):
+ with runtime_config_lock:
+ try:
+ reloaded = type(llm_settings)()
+ _restore_config(llm_settings, reloaded.model_dump())
Review Comment:
⚠️ Blocking: no. Summary: This reload assigns each field on the shared
llm_settings object under runtime_config_lock, but provider construction in
init_llm.py reads that object without taking the lock. A request overlapping a
page-load refresh can observe the new provider type with old model, endpoint,
or credential values and fail to construct its client. Please make provider
creation use a consistent settings snapshot synchronized with reloads, and add
a focused concurrent read/reload regression check. Evidence: _restore_config
sets fields sequentially at this line; get_chat_llm and the other provider
constructors read type and configuration fields separately in
hugegraph_llm/models/llms/init_llm.py.
##########
hugegraph-llm/src/hugegraph_llm/config/models/base_config.py:
##########
@@ -150,15 +162,9 @@ def _sync_object_to_env(self, env_config, config_dict):
def __init__(self, **data):
try:
file_exists = os.path.exists(env_path)
- # Step 1: Load environment variables if file exists
- if file_exists:
- env_config = dotenv_values(env_path)
- for k, v in env_config.items():
- os.environ[k] = v
-
- # Step 2: Init the parent class with loaded environment variables
+ # Load and validate settings directly from the current sources.
Review Comment:
⚠️ Blocking: no. Summary: Removing the .env-to-os.environ copy also drops
HUGEGRAPH_LLM_ENABLE_THIN_WRITES: thin_api._thin_write_disabled() still reads
it with os.getenv, but this key is not a BaseConfig field and is ignored by the
settings source. A deployment that stores this documented flag only in .env
will leave POST /graph-import and /vid-embeddings/refresh disabled. Please load
this standalone flag through a settings field or dotenv lookup, and add a
regression test without exporting it into the process environment. Evidence:
BaseConfig.__init__ no longer publishes arbitrary .env keys; thin_api.py:115
reads this flag directly from the process environment;
hugegraph-llm/README.md:198 documents the opt-in.
--
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]
