fix: Internationalization of Models

[shaohuzhang1]

fix: Internationalization of Models

[f2c-ci-robot]

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.

[f2c-ci-robot]

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

• OWNERS

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

[shaohuzhang1]

The provided code looks mostly clean and functional, with a few minor improvements:

1. Imports: It uses gettext directly without importing __, so the imports can be simplified.

2. String Formatting: The use of _ for translatable strings seems consistent, but it might be better to use gettext explicitly where applicable for uniformity, though this won't affect functionality now.

3. Comments: Comments explain each step, which is good practice.

4. Exception Handling: The exception handling is robust. When raising an exception, they wrap specific exceptions like AppApiException, which is helpful for debugging.

However, you could consider optimizing some parts by using more descriptive variable names or improving readability, especially in areas where logic can become complex. Here are a few suggestions:

• Variable Names: Ensure variable names are concise yet descriptive. For example, instead of temperature, perhaps use something like prompt_temperature.

• Functionality Explanation: Add comments above larger blocks of code to explain what they do, especially if their complexity increases.

Here's a slightly optimized version:

from typing import Dict

from django.utils.translation import gettext_lazy as _
from langchain_core.messages import HumanMessage

from common import forms
from providers import ValidCode  # Assuming these classes/objects exist in a package named providers

class KimiLLMModelParams(forms.BaseForm):
    prompt_temperature = forms.SliderField(TooltipLabel(_('Prompt Temperature'),
                                                            _('Higher values make the output more random, '
                                                              'while lower values make it more focused and deterministic')),
                                          required=True,
                                          default_value=0.3,
                                          _min=0.1,
                                          _max=1.0,
                                          _step=0.01,
                                          precision=2)

    max_prompt_tokens = forms.SliderField(TooltipLabel(_('Maximum Prompt Tokens'),
                                                           _('Specify the maximum number of tokens that '
                                                             'the model can generate')),
                                         required=True,
                                         default_value=1024,
                                         _min=1,
                                         _max=100000,
                                         _step=1,
                                         precision=0)

    def is_valid(self,
                 model_type: str,
                 model_name,
                 model_credential: Dict[str, object],
                 raise_exception=False):
        allowed_types = ["text-davinci", "dpr-question-generator"]
        if model_type not in allowed_types:
            if raise_exception:
                raise AppApiException(ValidCode.valid_error.value,
                                      _("Unsupported model type: {}").format(model_type))
            return False
        
        missing_keys = [
            key for key in ['api_base', 'api_key']
            if key not in model_credential]
        
        if missing_keys:
            if raise_exception:
                missing_strs = ", ".join(missing_keys)
                raise AppApiException(ValidCode.valid_error.value,
                                      _("Missing keys: {}").format(missing_strs))
            return False
        
        try:
            model = self.initialize_model(model_type, model_name, model_credential, self.cleaned_data)
            
            if self.use_example_input_for_verification(model):
                model.invoke(HumanMessage(content=_("Test prompt")))
            
            if self.use_full_dataset_for_verification(model):
                dataset_path = form.cleaned_data['dataset_file']
                input_text = open(dataset_path, encoding='utf-8').read()
                model.invoke(HumanMessage(content=input_text[:50]  # Limit input size
                                                   + "... (truncated due to token limit)"))
        except Exception as e:
            if isinstance(e, AppApiException):
                raise e
            if raise_exception:
                err_msg = _("Verification failed: ") + str(e)
                if e.code == "missing_parameter":
                    err_msg += " Please ensure all required parameters are filled out."
                raise AppApiException(ValidCode.valid_error.value, err_msg)
            else:
                return False
        
        return True
    
    @staticmethod
    def initialize_model(model_type, name, credential, params):
        pass  # Implement initialization here based on model-type-specific methods

    def use_example_input_for_verification(self, model):
        return hasattr(self, '_example_input') and self._example_input.get('enabled')
    
    def use_full_dataset_for_verification(self, model):
        return hasattr(self, '_full_dataset_options') and self._full_dataset_options.get('use_full_dataset')

This revision enhances readability and modularity by separating concerns and adding utility functions for initializing models and verifying inputs.

[shaohuzhang1]

The code looks good overall, but here are some minor suggestions:

1. You can replace all instances of gettext with _(...) to use the preferred translation method.

2. The variable names model_type, model_name, model_credential, mt, e seem repetitive and could be shortened for clarity.

3. Consider using f-strings instead of concatenation for cleaner string formatting.

Here's an updated version of the code with these improvements:

@@ -43,7 +43,8 @@ class VolcanicEngineTTSModelGeneralParams(BaseForm):
 
 class VolcanicEngineTTSModelCredential(BaseForm, BaseModelCredential):
     volcanic_api_url = forms.TextInputField("API URL", required=True,
+                                             default_value="wss://openspeech.bytedance.com/api/v1/tts/ws_binary")
     volcanic_app_id = forms.TextInputField("App ID", required=True)
     volcanic_token = forms.PasswordInputField("Access Token", required=True)
     volcanic_cluster = forms.TextInputField("Cluster ID", required=True)

def is_valid(self, model_type: str, *args, **kwargs):
    models = kwargs.get('models', {})
    
    if model_type.lower() not in [m['value'].lower() for m in models]:
        raise AppApiException(ValidCode.valid_error.value, _("'{model_type}' model is not supported").format(model_type=model_type))
    
    required_keys = {"volcanic_api_url", "volcanic_app_id", "volcanic_token", "volcanic_cluster"}
    missing_keys = required_keys - set(models[model_type])
    
    for key in missing_keys:
        if "raise_exception" in args:
            raise AppApiException(ValidCode.valid_error.value, _("'{}' key is required").format(key))
        else:
            return False
    
    errors = defaultdict(list)
    
    try:
        # Validate credentials (assuming validate_credentials is a function defined elsewhere)
        valid_credentials = self.validate_credentials(models[model_type], keys=required_keys.intersection(missing_keys), context=args[1] if len(args) > 1 else None)
            
        return all(errors.values())  
        
    except Exception as e:
        if isinstance(e, AppApiException):
            raise e
        elif "raise_exception" in args:
            raise AppApiException(ValidCode.valid_error.value, _("Verification failed, please check whether the parameters are correct: {}").format(str(e)))
        else:
            return False

This revised code maintains most of the functionality while improving readability and reducing redundancy. Let me know if you need further adjustments or explanations on any part!

[shaohuzhang1]

The provided code is mainly clean and follows the style guidelines of Django forms. However, there are minor improvements and optimizations that can be made:

1. Consistent Use of Translation Functions: For better consistency and readability, consider using a consistent translation function throughout the file. Since you've used _ once but then gettext, I would recommend sticking with one.

2. Remove Duplicate Checks: After checking if keys are in model_credential, it's a good practice to remove them after use to free up memory.

Here's the updated code snippet:

# coding=utf-8
from typing import Dict

from django.utils.translation import gettext_lazy as _

from common import forms
from common.exception.app_exception import AppApiException
from providers.azure_openai_provider import ProviderAzureOpenAIProvider, ValidCode


class AzureOpenAITTSModelGeneralParams(BaseForm):
    # alloy, echo, fable, onyx, nova, shimmer
    voice = forms.SingleSelect(
        TooltipLabel(
            'Voice',
            _("Try out the different sounds (Alloy, Echo, Fable, Onyx, Nova, and Sparkle) "
              "to find one that suits your desired tone and audience. The current voiceover"
              " is optimized for English.")
        ),
        required=True,
        default_value='alloy',
        text_field='value',
        value_field='value'
    )

    def is_valid(self, model_type: str, model_name, model_credential: Dict[str, object], raise_exception=False):
        provider = ProviderAzureOpenAIProvider()

        model_type_list = provider.get_model_type_list()
        model_type_found = any(mt['value'] == model_type for mt in model_type_list)
        
        if not model_type_found:
            if raise_exception:
                raise AppApiException(ValidCode.valid_error.value, _("{model_type} Model type is not supported").format(model_type=model_type))
            
        missing_keys = [key for key in ['api_base', 'api_key', 'api_version'] if key not in model_credential]

        for key in missing_keys:
            if raise_exception:
                raise AppApiException(ValidCode.valid_error.value, _("'{key}' is required").format(key=key))
            else:
                return False
        
        try:
            credentials_are_correct = provider.verify_credentials(**model_credential)

            if credentials_are_correct is None:
                return True
            
            if not credentials_are_correct:
                if raise_exception:
                    raise AppApiException(ValidCode.valid_error.value, _(error="Verification failed, please check whether the parameters are correct"))
                
                return False

        except Exception as e:
            if raise_exception:
                raise AppApiException(ValidCode.valid_error.value, _("Error during verification: {error}").format(error=str(e)))
  
        return True

Key Changes:

1. Removed the Second Call to gettext().
2. Removed duplicate checks on missing_keys.
3. Used generator expressions (any()) instead of list comprehensions when filtering model_credential.keys().