Conversation
--story=1017838 --user=王孝刚 【模型设置】支持腾讯云的大语言模型 issue#2230 https://www.tapd.cn/57709429/s/1653533
|
Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
| api_key = forms.PasswordInputField('API Key', required=True) | ||
|
|
||
| def get_model_params_setting_form(self, model_name): | ||
| return TencentCloudLLMModelParams() |
There was a problem hiding this comment.
The code appears to be properly structured, including importing necessary modules and defining classes with specific functionalities. However, here are a few improvements and points to consider:
Improvements:
-
Whitespace Consistency: The code uses inconsistent indentation levels (4 spaces vs. tabs). It's recommended to stick with one style consistently.
-
Comments and Formatting: Ensure comments explain complex logic or decisions clearly. Also, format functions and variables using PEP8 conventions for better readability.
-
Docstring Style: Use Google-style docstrings instead of Django-style doc strings for consistency.
-
Error Handling: Consider adding more detailed logging around exceptions to help with debugging.
-
Security Practices: Always ensure sensitive information like API keys is handled securely, especially when they're being used to interact with external services.
-
Type Hints: Although Python 3 supports type hints well, it might be helpful to include them in all class methods where applicable.
-
Testing: Add unit tests to cover different scenarios, such as valid and invalid credentials, model invocation failures, etc.
Here are some example adjustments:
# Importing necessary modules
from typing import Dict
from django.utils.translation import gettext_lazy as _, gettext
from langchain_core.messages import HumanMessage
from common import forms
from common.exception.app_exception import AppApiException
from common.forms import BaseForm, TooltipLabel
from setting.models_provider.base_model_provider import BaseModelCredential, ValidCode
class TencentCloudLLMModelParams(BaseForm):
"""
Parameters for the Tencent Cloud LLM model.
"""
temperature = forms.SliderField(
TooltipLabel(_('Temperature'), _('Higher values make the output more random, while lower values make it more focused and deterministic')),
required=True,
default_value=0.7,
_min=0.1,
_max=1.0,
_step=0.01,
precision=2
)
max_tokens = forms.SliderField(
TooltipLabel(_('Output the maximum Tokens'),
_('Specify the maximum number of tokens that the model can generate')),
required=True,
default_value=800,
_min=1,
_max=100000,
_step=1,
precision=0
)
class TencentCloudLLMModelCredential(BaseForm, BaseModelCredential):
def __init__(self, *args, provider=None, **kwargs):
self.provider = provider
super().__init__(*args, **kwargs)
def is_valid(self, model_type: str, model_name, model_credential: Dict[str, object], model_params, provider,
raise_exception=False) -> bool:
"""
Validates the Tencent Cloud LLM model credential.
:param model_type: Type of the model
:param model_name: Name of the model
:param model_credential: Dictionary containing model credentials
:param model_params: Parameter settings for the model
:param provider: Model provider instance
:param raise_exception: Flag indicating whether to raise an exception in case of validation failure
:return: Boolean indicating whether the credentials are valid
"""
if not any(list(filter(lambda mt: mt.get('value') == model_type, provider.get_model_type_list()))):
raise AppApiException(ValidCode.valid_error.value,
gettext('{model_type} Model type is not supported').format(model_type=model_type))
required_keys = ['api_base', 'api_key']
missing_keys = [key for key in required_keys if key not in model_credential]
if missing_keys:
msg = "Required fields '{}' are missing.".format(", ".join(missing_keys))
if raise_exception:
raise AppApiException(ValidCode.valid_error.value, msg)
else:
return False
try:
model = provider.get_model(model_type, model_name, model_credential, **model_params)
model.invoke([HumanMessage(content=gettext('Hello'))])
except AppApiException as e:
if isinstance(e, AppApiException):
raise e
elif raise_exception:
raise AppApiException(ValidCode.valid_error.value,
gettext(
'Verification failed, please check whether the parameters are correct: {error}').format(
error=str(e)))
else:
return False
return True
def encryption_dict(self, model: Dict[str, object]) -> Dict[str, object]:
"""
Encrypts sensitive information from the model dictionary.
:param model: Original model dictionary
:return: Encrypted model dictionary
"""
return {**model, 'api_key': super(TencentCloudLLMModelCredential, self).encryption(model.get('api_key', ''))}
@property
def api_base(self) -> str:
"""API base URL."""
return self.cleaned_data['api_base']
@property
def api_key(self) -> str:
"""API key."""
return self.cleaned_data['api_key']
def get_model_params_setting_form(self, model_name) -> TencentCloudLLMModelParams:
"""
Returns the form for configuring model parameters.
:param model_name: Name of the model
:return: Form for configuring model parameters
"""
return TencentCloudLLMModelParams()
These changes improve code maintainability, readability, and security practices while adhering to best coding standards.
| return ModelProvideInfo(provider='model_tencent_cloud_provider', name=_('Tencent Cloud'), icon=get_file_content( | ||
| os.path.join(PROJECT_DIR, "apps", "setting", 'models_provider', 'impl', 'tencent_cloud_model_provider', | ||
| 'icon', | ||
| 'tencent_cloud_icon_svg'))) |
There was a problem hiding this comment.
There is one potential issue: The model_info_list variable is not being assigned to anything, which might cause an error in downstream operations if used. You can fix this by assigning it directly within the ModelInfoManage.builder() call:
model_info_manage = (
ModelInfoManage.builder()
.append_model_info_list([
ModelInfo('deepseek-v3', '', ModelTypeConst.LLM,
openai_llm_model_credential, TencentCloudChatModel
),
ModelInfo('deepseek-r1', '', ModelTypeConst.IMAGE,
openai_llm_model_credential, TencentCloudChatModel
)
])
.append_default_model_info(
ModelInfo('deepseek-v3', _('The latest gpt-3.5-turbo, updated with OpenAI adjustments'), ModelTypeConst.LLM,
openai_llm_model_credential, TencentCloudChatModel)
)
.build()
)
| return super().get_num_tokens(text) | ||
| except Exception as e: | ||
| tokenizer = TokenizerManage.get_tokenizer() | ||
| return len(tokenizer.encode(text)) |
There was a problem hiding this comment.
Here are the potential improvements and corrections to your Python code:
-
The
@staticmethoddecorator should be used beforenew_instance, not inside it. -
The constructor parameters should match those expected by base classes, but there is no clear implementation for them yet, assuming they're not necessary or will be added later.
-
It seems like
get_num_tokens_from_messages()calls a private method_tokenize_chat_message()that does not exist in this class. This might need further investigation if needed. -
Instead of using raw strings for docstrings (e.g.,
"""..."""), use triple quotes without whitespace at ends ('''...''') which is more pythonic. -
The function names could be more descriptive.
-
Consider adding error handling when encoding messages with the tokenizer. For example:
messages = [BaseMessage(content="Hello")] # Example message instance
try:
num_tokens = maxkb.model.TencentCloudChatModel.get_num_tokens_from_messages(messages)
print(f"Number of tokens: {num_tokens}")
except ValueError as ve:
print("Tokenization error:", ve)-
Ensure that the imported modules (
langchain_coreetc.) have valid imports for the versions installed in your environment. -
Add type hints where possible, especially on functions and variables where their types are crucial.
feat: support Tencent Cloud --story=1017838 --user=王孝刚 【模型设置】支持腾讯云的大语言模型 issue#2230 https://www.tapd.cn/57709429/s/1653533