Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: BaiLian Image Model #1844

Merged
merged 1 commit into from
Dec 16, 2024
Merged

feat: BaiLian Image Model #1844

merged 1 commit into from
Dec 16, 2024

Conversation

shaohuzhang1
Copy link
Contributor

feat: BaiLian Image Model

Copy link

f2c-ci-robot bot commented Dec 16, 2024

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.

Instructions 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.

Copy link

f2c-ci-robot bot commented Dec 16, 2024

[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.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

api_key = forms.PasswordInputField('API Key', required=True)

def get_model_params_setting_form(self, model_name):
return QwenModelParams()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

代码中存在以下不规范和潜在问题:

  1. from typing import Dict 应该是 from typing import * 来涵盖所有内置类型的导入。

  2. from common.forms import BaseForm, TooltipLabel 中的 BaseFormTooltipLabel 可能会与其他模块中的同名类冲突或重命名需求。

  3. QwenModelParams 类中的 _min, _max", 和 _step` 属性在 Python 3 中不需要使用下划线前缀。这些属性用于定义滑块组件的最小、最大和步长范围。如果需要支持旧版本,请保持不变;如需更新,可以修改为无前导下划线的形式。

  4. QwenTextToImageModelCredential 类中,方法参数顺序应与实际使用的顺序一致(即:raise_exception 参数应该放在最前面)。

  5. encryption_dict 方法中,对于 API 密钥(api_key)进行加密,并返回完整的解密后的模型字典(包含其他键值对)。这会导致任何依赖原始未加密 API 密钥的功能失效。建议只在其必要情况下进行加密操作。

优化建议:

  • 尽量减少不必要的字段,默认值设置得更为合理。
  • 异常处理逻辑中,当验证失败时返回具体的错误信息以帮助用户调试。
  • 确保 get_model_params_setting_form 方法返回的是正确的表单对象实例。

api_key = forms.PasswordInputField('API Key', required=True)

def get_model_params_setting_form(self, model_name):
return QwenModelParams()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

代码整体结构良好,但存在一些需要关注的问题和优化点:

  1. 导入顺序问题

    from langchain_core.messages import HumanMessage

    应该先导入 HumanMessage 类。

  2. API 错误处理

    except Exception as e:
        if isinstance(e, AppApiException):
            raise e
        if raise_exception:
            raise AppApiException(ValidCode.valid_error.value, f'校验失败,请检查参数是否正确: {str(e)}')
        else:
            return False

    在捕获到其他异常时,建议只打印日志或记录错误信息,而不是重新抛出异常,除非有明确的需求。

  3. 加密逻辑简化

    def encryption_dict(self, model: Dict[str, object]):
        return {**model, 'api_key': super().encryption(model.get('api_key', ''))}

    如果 BaseModelCredential 类已经实现了一个安全的加密方法,并且这个加密逻辑适用于 QwenVLModelCredential,可以直接调用而不需要重复编写加密。

  4. 模型测试功能

    try:
        model = provider.get_model(model_type, model_name, model_credential)
        res = model.stream([HumanMessage(content=[{"type": "text", "text": "你好"}])])
        for chunk in res:
            print(chunk)
    except Exception as e:
        if isinstance(e, AppApiException):
            raise e
        if raise_exception:
            raise AppApiException(ValidCode.valid_error.value, f'校验失败,请检查参数是否正确: {str(e)}')
        else:
            return False

    需要添加调试信息以确保模型流获取成功,或者减少打印过多的日志以便于后续维护。

  5. 类成员顺序

    class QwenVLModelCredential(BaseForm, BaseModelCredential):
        api_key = forms.PasswordInputField('API Key', required=True)
        # 其他成员...

    推荐遵循驼峰命名法(PascalCase)来定义类成员名称,例如使用 apiKey 而不是 api_key

  6. 注释文档

    """
    @project: MaxKB
    @Author:虎
    @file: llm.py
    @date:2024/7/11 18:41
    @desc:
    """

通过上述改进,可以提高代码质量,避免潜在的安全风险和性能问题。

else:
print('sync_call Failed, status_code: %s, code: %s, message: %s' %
(rsp.status_code, rsp.code, rsp.message))
return file_urls
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

看起来这段代码是一个从阿里云迁移到DashScope的文本到图像模型实现。以下是存在的一些问题和改进建议:

  1. 缺少必要的导入: 还需要dashscope.ImageSynthesis模块和HTTPStatus类的导入。
  2. 不必要的方法调用: check_auth方法中没有实际的操作来验证身份认证,且不需要每次都创建一个全新的聊天对象。
  3. 参数默认值: 参数在构造函数初始化时已经覆盖了静态方法中的默认参数,这可能不是有意为之的。
@@ -0,0 +1,58 @@
+# coding=utf-8
+from http import HTTPStatus
+from typing import Dict
+
+from dashscope.dashscope_client import DashScopeClient  # 修正缺失导入
+from dashscope import ImageSynthesis
+from langchain_community.chat_models import ChatTongyi
+from langchain.core.messages import HumanMessage
+
+from setting.models_provider.base_model_provider import MaxKBBaseModel
+from setting.models_provider.impl.base_tti import BaseTextToImage
+
+
+class QwenTextToImageModel(MaxKBBaseModel, BaseTextToImage):
+    api_key: str
+    model_name: str
+    params: dict
    

以上是主要的问题和改进建议,可以根据实际情况进一步调整其他部分。

@liuruibin liuruibin merged commit 3303640 into main Dec 16, 2024
4 of 5 checks passed
@liuruibin liuruibin deleted the pr@main@refactor_bailian_model branch December 16, 2024 08:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants