You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hugegraph.apache.org by "ChenZiHong-Gavin (via GitHub)" <gi...@apache.org> on 2024/12/02 14:01:12 UTC
[PR] refactor(llm): use pydantic-settings for config management [incubator-hugegraph-ai]
ChenZiHong-Gavin opened a new pull request, #122:
URL: https://github.com/apache/incubator-hugegraph-ai/pull/122
(no comment)
--
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] refactor(llm): use pydantic-settings for config management [incubator-hugegraph-ai]
Posted by "imbajin (via GitHub)" <gi...@apache.org>.
imbajin merged PR #122:
URL: https://github.com/apache/incubator-hugegraph-ai/pull/122
--
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] refactor(llm): use pydantic-settings for config management [incubator-hugegraph-ai]
Posted by "returnToInnocence (via GitHub)" <gi...@apache.org>.
returnToInnocence commented on PR #122:
URL: https://github.com/apache/incubator-hugegraph-ai/pull/122#issuecomment-2524129522
There seems to be a problem with the configuration loading in the image above, which I assume is a legacy issue from the previous version and could be fixed in this PR as well
The location in the code is as follows
--
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] refactor(llm): use pydantic-settings for config management [incubator-hugegraph-ai]
Posted by "ChenZiHong-Gavin (via GitHub)" <gi...@apache.org>.
ChenZiHong-Gavin commented on PR #122:
URL: https://github.com/apache/incubator-hugegraph-ai/pull/122#issuecomment-2520799874
After using lib pydantic-settings, when we use the base model `Config`, the selected value will be determined in the following order of priority (from highest to lowest):
1. Parameters passed in the CLI if cli_parse_args is enabled.
2. Parameters passed to the Settings class initializer.
3. Environment variables
4. Variables loaded from the dotenv (.env) file.
5. Variables loaded from the secrets directory.(We may make api_key secret later maybe)
6. Default field values of the Config model.
refer: https://docs.pydantic.org.cn/latest/concepts/pydantic_settings/#field-value-priority
--
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] refactor(llm): use pydantic-settings for config management [incubator-hugegraph-ai]
Posted by "returnToInnocence (via GitHub)" <gi...@apache.org>.
returnToInnocence commented on code in PR #122:
URL: https://github.com/apache/incubator-hugegraph-ai/pull/122#discussion_r1872523513
##########
hugegraph-llm/src/hugegraph_llm/config/__init__.py:
##########
@@ -16,15 +16,21 @@
# under the License.
-__all__ = ["settings", "resource_path"]
+__all__ = ["huge_settings", "admin_settings", "llm_settings", "resource_path"]
import os
-from .config import Config, PromptConfig
-settings = Config()
Review Comment:
There is a small issue here, which is that if the settings are changed to multiple subcategories of settings, the corresponding functionality for other application settings also needs to be adjusted accordingly. Otherwise, the issue shown in the image below will occur.
The specific code that causes the issue is shown in the image below.
--
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] refactor(llm): use pydantic-settings for config management [incubator-hugegraph-ai]
Posted by "returnToInnocence (via GitHub)" <gi...@apache.org>.
returnToInnocence commented on code in PR #122:
URL: https://github.com/apache/incubator-hugegraph-ai/pull/122#discussion_r1873936021
##########
hugegraph-llm/src/hugegraph_llm/config/llm_config.py:
##########
@@ -0,0 +1,97 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+
+import os
+
+from typing import Optional, Literal
+from .models import BaseConfig
+
+
+class LLMConfig(BaseConfig):
+ """LLM settings"""
+
+ chat_llm_type: Literal["openai", "ollama/local", "qianfan_wenxin", "zhipu"] = "openai"
+ extract_llm_type: Literal["openai", "ollama/local", "qianfan_wenxin", "zhipu"] = "openai"
+ text2gql_llm_type: Literal["openai", "ollama/local", "qianfan_wenxin", "zhipu"] = "openai"
+ embedding_type: Optional[Literal["openai", "ollama/local", "qianfan_wenxin", "zhipu"]] = "openai"
+ reranker_type: Optional[Literal["cohere", "siliconflow"]] = None
+ # 1. OpenAI settings
+ openai_chat_api_base: Optional[str] = os.environ.get("OPENAI_BASE_URL", "https://api.openai.com/v1")
Review Comment:
There seems to be a problem with the environment variable that can't be found in env. I guess there is an error in the name, so it can't be read in properly. I guess it is "OPENAI_API_BASE"?
In the system, the configuration does not load properly
--
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] refactor(llm): use pydantic-settings for config management [incubator-hugegraph-ai]
Posted by "ChenZiHong-Gavin (via GitHub)" <gi...@apache.org>.
ChenZiHong-Gavin commented on code in PR #122:
URL: https://github.com/apache/incubator-hugegraph-ai/pull/122#discussion_r1872538038
##########
hugegraph-llm/src/hugegraph_llm/config/__init__.py:
##########
@@ -16,15 +16,21 @@
# under the License.
-__all__ = ["settings", "resource_path"]
+__all__ = ["huge_settings", "admin_settings", "llm_settings", "resource_path"]
import os
-from .config import Config, PromptConfig
-settings = Config()
Review Comment:
we may first test the `Config` class in `hugegraph-llm/src/hugegraph_llm/config/generate.py`.
after that, we can replace those uses.
--
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] refactor(llm): use pydantic-settings for config management [incubator-hugegraph-ai]
Posted by "ChenZiHong-Gavin (via GitHub)" <gi...@apache.org>.
ChenZiHong-Gavin commented on code in PR #122:
URL: https://github.com/apache/incubator-hugegraph-ai/pull/122#discussion_r1872538038
##########
hugegraph-llm/src/hugegraph_llm/config/__init__.py:
##########
@@ -16,15 +16,21 @@
# under the License.
-__all__ = ["settings", "resource_path"]
+__all__ = ["huge_settings", "admin_settings", "llm_settings", "resource_path"]
import os
-from .config import Config, PromptConfig
-settings = Config()
Review Comment:
we may first test the `Config` class in `hugegraph-llm/src/hugegraph_llm/config/generate.py`
--
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] refactor(llm): use pydantic-settings for config management [incubator-hugegraph-ai]
Posted by "returnToInnocence (via GitHub)" <gi...@apache.org>.
returnToInnocence commented on code in PR #122:
URL: https://github.com/apache/incubator-hugegraph-ai/pull/122#discussion_r1872523513
##########
hugegraph-llm/src/hugegraph_llm/config/__init__.py:
##########
@@ -16,15 +16,21 @@
# under the License.
-__all__ = ["settings", "resource_path"]
+__all__ = ["huge_settings", "admin_settings", "llm_settings", "resource_path"]
import os
-from .config import Config, PromptConfig
-settings = Config()
Review Comment:
There is a small issue here, which is that if the settings are changed to multiple subcategories of settings, the corresponding functionality for other application settings also needs to be adjusted accordingly. Otherwise, the issue shown in the image below will occur.
The specific code that causes the issue is shown in the image below.
--
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] refactor(llm): use pydantic-settings for config management [incubator-hugegraph-ai]
Posted by "returnToInnocence (via GitHub)" <gi...@apache.org>.
returnToInnocence commented on code in PR #122:
URL: https://github.com/apache/incubator-hugegraph-ai/pull/122#discussion_r1872528594
##########
hugegraph-llm/src/hugegraph_llm/config/__init__.py:
##########
@@ -16,15 +16,21 @@
# under the License.
-__all__ = ["settings", "resource_path"]
+__all__ = ["huge_settings", "admin_settings", "llm_settings", "resource_path"]
import os
-from .config import Config, PromptConfig
-settings = Config()
Review Comment:
There is a small issue here, which is that if the settings are changed to multiple subcategories of settings, the corresponding functionality for other application settings also needs to be adjusted accordingly. Otherwise, the issue shown in the image below will occur.
The specific code that causes the issue is shown in the image below.
--
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] refactor(llm): use pydantic-settings for config management [incubator-hugegraph-ai]
Posted by "returnToInnocence (via GitHub)" <gi...@apache.org>.
returnToInnocence commented on code in PR #122:
URL: https://github.com/apache/incubator-hugegraph-ai/pull/122#discussion_r1873936021
##########
hugegraph-llm/src/hugegraph_llm/config/llm_config.py:
##########
@@ -0,0 +1,97 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+
+import os
+
+from typing import Optional, Literal
+from .models import BaseConfig
+
+
+class LLMConfig(BaseConfig):
+ """LLM settings"""
+
+ chat_llm_type: Literal["openai", "ollama/local", "qianfan_wenxin", "zhipu"] = "openai"
+ extract_llm_type: Literal["openai", "ollama/local", "qianfan_wenxin", "zhipu"] = "openai"
+ text2gql_llm_type: Literal["openai", "ollama/local", "qianfan_wenxin", "zhipu"] = "openai"
+ embedding_type: Optional[Literal["openai", "ollama/local", "qianfan_wenxin", "zhipu"]] = "openai"
+ reranker_type: Optional[Literal["cohere", "siliconflow"]] = None
+ # 1. OpenAI settings
+ openai_chat_api_base: Optional[str] = os.environ.get("OPENAI_BASE_URL", "https://api.openai.com/v1")
Review Comment:
There seems to be a problem with the environment variable that can't be found in env. I guess there is an error in the name, so it can't be read in properly. I guess it is "OPENAI_API_BASE"?
--
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