-
Notifications
You must be signed in to change notification settings - Fork 2.6k
fix: XF tts model #4486
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
fix: XF tts model #4486
Conversation
|
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-sigs/prow 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 |
| _min=1, | ||
| _max=100, | ||
| _step=5, | ||
| precision=1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here are some comments and suggestions to optimize or address potential issues in the provided code:
# The file should be saved with UTF-8 encoding
# Line 1: Add necessary imports at the top of the file
# Class definitions for different TTS models from Xunfei
class XunFeiDefaultTTSModelCredential(BaseForm, BaseModelCredential):
"""
Factory class credential for Xunfei TTS based on api_version routing to specific implementations.
"""
# api_version field definition using SingleSelectField
api_version = forms.SingleSelect(
_("API Version"),
required=True,
text_field='label',
value_field='value',
default_value='online',
option_list=[
{'label': _('Online TTS'), 'value': 'online'},
{'label': _('Super Humanoid TTS'), 'value': 'super_humanoid'}
])
# Additional fields defined here
spark_api_url = forms.TextInputField(_('API URL'), required=True)
spark_app_id = forms.TextInputField('APP ID', required=True)
spark_api_key = forms.PasswordInputField(_("API Key"), required=True)
spark_api_secret = forms.PasswordInputField('API Secret', required=True)
def is_valid(self, model_type: str, model_name, model_credential: Dict[str, object], model_params, provider,
raise_exception=False):
"""
Validates the model credentials based on API version and other fields.
Raises exceptions if validation fails.
:param:model_type:str Type of the model
:param:model_name:str Name of the model
:param:model_credential:Dict[str, object] Credentials for the model
:param:model_params:dict Parameters passed to the model
:param:provider:XunFeiProvider Instance of the provider
:param:raise_exception,bool Flag to indicate raising exception if validation fails
"""
model_types_list = provider.get_model_type_list()
valid_models = [mt['value'] for mt in model_types_list]
if model_type not in valid_models:
message = gettext('{_type} Model type is not supported').format(_type=model_type)
raise AppApiException(ValidCode.valid_error.value, message)
api_version = model_credential.get('api_version', 'online')
missing_fields = [
key for key in ['spark_api_url', 'spark_app_id', 'spark_api_key', 'spark_api_secret']
if key not in model_credential
]
if missing_fields and len(missing_fields) > 0:
msg = gettext('{} is required').format(','.join(missing_fields))
raise AppApiException(ValidCode.valid_error.value, msg if raise_exception else False)
try:
model_instance = provider.get_model(model_type, model_name, model_credential, **model_params)
model_instance.check_auth()
except Exception as e:
logging.error(f'Error while validating model: {e}')
logging.debug(str(e), exc_info=True)
if isinstance(e, AppApiException):
raise e
if raise_exception:
message = gettext(
"Verification failed, please check whether the parameters are correct: {}".format(str(e)))
raise AppApiException(ValidCode.valid_error.value, message)
else:
return False
return True
def encryption_dict(self, model_dict):
"""
Encrypts sensitive fields (like spark_api_secret).
"""
if not model_dict.get('spark_api_secret'):
model_dict.pop('spark_api_secret')
encrypted_model_dict = super().encryption(model_dict)
if 'spark_api_secret' in encrypted_model_dict:
model_dict['spark_api_secret'] = encrypted_model_dict.pop('spark_api_secret')
return encrypted_model_dict
class XunFeiDefaultTTSModelParams(BaseForm):
"""
Form containing general parameters for the factory class.
Only includes common parameters that don't require a VCN token.
"""
speed = SliderField(
TooltipLabel(_("speaking speed"), _('Speech speed, optional value: [0-100], default is 50')),
min=_min=1,
max=_max=100,
step=_step=5,
precision=1)Optimizations and Suggestions
-
Encoding: Ensure the
.pyfile has the UTF-8 encoding (# coding=utf-8). This is crucial for handling non-ASCII characters. -
Imports: Import
logging, which will enable you to log errors instead of relying solely on Django's logging module for more detailed debugging information. -
Detailed Docstrings: Provide comprehensive docstring explanations for methods like
is_validand method arguments. -
Logging Errors: Use Python’s built-in logging functionality instead of relying only on MaxKBLogger.
-
Handling Missing Fields: Instead of returning
False, return an empty dictionary orNone. It’s generally better to handle data integrity issues when they occur rather than allowing invalid input through. -
Encryption Logic: Simplify the encryption logic to avoid unnecessary popping operations and ensure all keys are encrypted properly before passing back the updated dictionary.
-
Variable Names Convention: Follow a consistent variable name convention throughout the codebase. Consistent naming improves readability and maintainability.
By following these recommendations, you can improve both the robustness and security of your TTS factory credential implementation.
fix: XF tts model