-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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: i18n #2039
feat: i18n #2039
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. 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. |
[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 |
allow_null=True, allow_blank=True) | ||
phone = serializers.CharField(required=False, error_messages=ErrMessage.char(_("Phone")), max_length=20, | ||
allow_null=True, allow_blank=True) | ||
|
||
def is_valid(self, *, raise_exception=True): | ||
super().is_valid(raise_exception=True) |
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.
The provided code snippet appears to be a set of serializers for handling HTTP requests related to users and their roles within an API application. Here are some observations:
Irregularities, Potential Issues, or Optimization Suggestions
-
Duplicated Code: The
get_response_body_api
method is duplicated across multiple places (in different classes). Consider consolidating this to avoid redundancy. -
Unused Imports in
RePasswordSerializer
:from utils.base.exceptions import AppApiException, ErrMessage
-
Redundancy in Error Messages:
- In
SwitchLanguageSerializer
, you repeat the same error messages under botherror_messages
keys and inline comments. This can lead to confusion and inconsistency. - Similar issues exist in other classes where duplicate or redundant error messages are present.
- In
-
String Interpolation in Email Content:
- You use string interpolation (
format()
) to insert variables into HTML content. While it works, consider using f-strings or.format()
with{}
syntax for better readability and maintainability.
- You use string interpolation (
-
Static Method
get_user_profile
Overriding Class Methodget_response_body_api
:- The
get_user_profile
static method overrides the existingget_response_body_api
class method. Ensure that this change aligns with your requirements and documentation.
- The
-
Validation Regex Errors:
- There are repeated regex patterns for validating passwords in different serializers without considering context-specific validation rules (e.g., password complexity constraints).
-
Inconsistent Use of Schema Types:
- Different serializers may have inconsistent use of schema types or descriptions, which can make them harder to understand and review collaboratively.
Recommendation
While the core logic looks functional, improving consistency and reducing duplication will help ensure code quality and ease future maintenance. Here are some specific recommendations:
Consolidate Duplicated Methods
def get_error_message(err_code, *args, **kwargs):
return _(ErrMessage.err_code_to_str_get_msg.get(err_code, '{}'))(*args, **kwargs)
# Usage:
err_message = get_error_message(AppErrorCodeConstants.USER_ID_INVALID.value.code)
Remove Redundant Error Messages
- Clean up any unnecessary error messages by combining those defined in the serializer's
error_messages
dictionary with inline explanations when necessary.
Simplify String Interpolations
html_message = f'<strong>{_("Hello")}</strong>'
Refactor Override Methods
Ensure that overwriting methods like get_user_profile
maintains clarity on how it differs from the original definition. For example:
class ImprovedUserProfileSerializer(UserProfileSerializer):
def get_user_profile(self, user: User):
# Add additional business logic here if needed
profile_data = {
"id": user.id,
# ... rest of the fields ...
}
return profile_data
By implementing these improvements, the codebase becomes more organized, readable, and maintainable.
'dataset_setting': ApplicationApi.DatasetSetting.get_request_body_api(), | ||
'model_setting': ApplicationApi.ModelSetting.get_request_body_api(), | ||
'problem_optimization': openapi.Schema(type=openapi.TYPE_BOOLEAN, title=_("Problem Optimization"), | ||
description=_("Problem Optimization"), default=True), | ||
'type': openapi.Schema(type=openapi.TYPE_STRING, title=_("Application Type"), | ||
description=_("Application Type SIMPLE | WORK_FLOW")), | ||
'problem_optimization_prompt': openapi.Schema(type=openapi.TYPE_STRING, title=_('Question optimization tips'), | ||
'problem_optimization_prompt': openapi.Schema(type=openapi.TYPE_STRING, | ||
title=_('Question optimization tips'), | ||
description=_("Question optimization tips"), | ||
default=_( | ||
"() contains the user's question. Answer the guessed user's question based on the context ({question}) Requirement: Output a complete question and put it in the <data></data> tag")), |
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.
The provided code snippet appears to be part of an OpenAPI schema definition for a REST API. Here are some observations and improvements:
Irregularities/Potential Issues
-
Duplicate
stt_model_enable
Property: The request body defines two instances of the same property ("stt_model_enable"
), which might not be intended. -
Missing Comma Before Final Property: In the
dataset_setting
,model_setting
, and other nested objects, there is no comma before the final property declaration, which may lead to syntax errors if used directly as JSON or Swagger files. -
Default String Formatting: The default strings contain newline characters at specific positions within
{data}
tags, which doesn't align with typical markdown formatting practices. -
Language Handling: There's a redundant mention of language handling in the
get_request_body_api()
function description, but it doesn't appear to be necessary or consistently implemented throughout. -
Prologue and Dialogue Number Documentation: These fields describe the opening remark ("prologue") and the number of multi-round conversations without specifying their types accurately (e.g., "number").
-
No Citation Segmentation Prompt Translation: The translation string for
"no_references_prompt"
seems incomplete or missing content.
Optimization Suggestions
-
Simplify Default Strings: Simplify the default strings for better readability. For example, remove unnecessary line breaks and ensure consistency in markdown usage.
-
Fix Duplicate Properties: Ensure only one instance of
"stt_model_enable"
is present. -
Add Missing Commas: Add commas between each parameter list item to improve clarity when used as JSON or YAML.
-
Consistent Language Usage: Clearly document how the answer should maintain consistency with the provided data and respect specified languages.
Here’s an optimized version of the code:
def get_request_body_api():
return {
'operationId': 'create_application',
'parameters': [
(
openai.Parameter('Request Body', 'body'),
None,
{}
),
],
'responses': {
},
'requestBody': {
'required': True,
'content': {
'application/json': {
'schema': {
'$ref': '#/components/schemas/CreateApplicationReq'
}
}
}
}
}
And the corresponding definition for CreateApplicationReq
schema:
def CreateApplicationReq_schema(name,
desc,
model_id,
dialogue_number,
dataset_setting,
model_setting,
problem_optimization=False,
type='SIMPLE',
stt_model_enable=True):
components = {"schemas": {}}
schemas = components['schemas']
applicationApiDatasetSetting = ApplicationApi.DatasetSetting.get_request_body_api()
applicationModelSetting = ApplicationApi.ModelSetting.get_request_body_api()
schemas["CreateApplicationReq"] = {
"type": "object",
"properties": {
"name": {"type": "string", "title": _("Application Name"), "description": _("Application Name")},
"desc": {"type": "string", "title": _("Application Description"), "description": _("Application Description")},
"model_id": {"type": "string", "title": _("Model ID"), "description": _("Model ID")},
"dialogue_number": {"type": "integer", "title": _("Number of Multi-Round Conversations"), "description": _("Number of Multi-Round Conversations")},
"dataset_ids_list": {"type": "array", "items": {"type": "string"}, "title": _("List of Associated Knowledge Base IDs"), "description": _("List of associated knowledge base IDs")},
"dataset_setting": {"$ref": "#/components/schemas/ApplicationApiDatasetSetting"},
"model_setting": {"$ref": "#/components/schemas/ApplicationModelSetting"},
"problem_optimization": {"type": "boolean", "title": _("Problem Optimization"), "description": _("Problem Optimization"), "default": False},
"type": {"type": "string", "title": _("Application Type"), "description": _("Application Type (SIMPLE | WORK_FLOW)"},
"problem_optimization_prompt": {"type": "string", "title": "_Question Optimization Tips_",
"description": _("Question optimization tips"),
"default": _("(_contains the user\'s question_) Output a complete question based on the context ({question}). Put it in the `<data></data>` tag")}
}
}
return components
@app.route("/v1/applications", methods=["POST"])
@RestController("/v1")
@api_response(code=201)
@endpoint_description("Creates a new application.")
@parameter(parameter="applicationJson", location="json", required=True,
description="JSON representation of the application configuration")
@ApiOperation(description="Create a new application.")
@RequestMapping(value="/applications", method={RequestMethod.POST})
public ResponseEntity createNewApplication(CreateApplication Req reqObject){
// Implementation logic here
return ResponseEntity.status(HttpStatus.CREATED).build();
}
class CreateApplicationReq{
private String name;
private String desc;
private String model_id;
private Integer dialogue_number;
private List<String> dataset_ids_list;
private ApplicationApiDatasetSetting dataset_setting;
private ApplicationModelSetting model_setting;
private Boolean problem_optimization;
private String type;
private String problem_optimization_prompt;
// getters and setters
}
Replace applicationApiDatasetSetting
and applicationModelSetting
with actual implementations or references as per your requirements. Adjust class names and properties according to your project design needs. This approach improves organization, maintainability, and readability while addressing the identified issues.
_(''' | ||
Phi-3 Mini is Microsoft's 3.8B parameter, lightweight, state-of-the-art open model. | ||
'''), | ||
_("Phi-3 Mini is Microsoft's 3.8B parameter, lightweight, state-of-the-art open model."), | ||
ModelTypeConst.LLM, ollama_llm_model_credential, OllamaChatModel), | ||
] | ||
ollama_embedding_model_credential = OllamaEmbeddingModelCredential() |
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.
The given code snippet appears to be configuring models in an application or service that supports various machine learning models from the Ollama library, which is developed by Anthropic. The configuration includes details such as model names, descriptions, types (LLM, Embedding), and credentials needed to access them.
Here are some observations and potential areas for improvement:
-
Duplicated Text: There seem to be multiple instances where similar text segments are repeated with slight minor variations. This can lead to redundancy and make maintenance more challenging. For example:
- Both "qwen:xxb" and "qwen2:xxb-instruct" have very similar descriptions.
-
Description Clarity: While most descriptions add value, ensuring clarity about each feature is crucial. Some paragraphs might omit essential information. For instance, it would help to specify whether these models support streaming output.
-
Consistent Format: Ensure consistency in formatting throughout the description sections. Using bullet points, indentation, or consistent capitalization can improve readability.
-
Optimization Suggestions:
- Consider consolidating redundant data if parts of one description can be applied universally across other entries.
- Evaluate if certain fields like
ModelTypeConst.LLM
,ollama_llm_model_credential
, andOllamaChatModel
are necessary in all configurations since they appear unchanged between different entries.
-
Error Handling: Make sure there’s error handling mechanism in place to manage missing or inappropriate credentials, especially those related to LLMs and embeddings.
-
User Feedback Mechanism: Implement a feedback loop so that users can report any issues or suggest improvements directly. This will enhance user satisfaction over time and reduce manual updates needed.
Overall, improving documentation quality and maintaining consistency can significantly enhance usability and reliability in your application.
feat: i18n