-
Notifications
You must be signed in to change notification settings - Fork 2.2k
feat: Swagger document response for adding OpenAI interface #2786
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
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 |
}) | ||
|
||
}))}) | ||
|
||
@staticmethod | ||
def get_request_body_api(): | ||
return openapi.Schema(type=openapi.TYPE_OBJECT, |
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.
Your code seems mostly clean with some improvements for clarity and readability:
-
Docstring: Add more detailed docstrings to describe what
get_response_body_api
does, especially since it's using placeholders like_
. This can help maintainers understand its purpose. -
Type Annotations: Consider adding type annotations to improve code quality. However, given Python 3.7+, you already have PEP 526 support which allows the use of typing aliases without explicit import statements.
-
Variable Naming: Variable names could be improved for better understanding. For example,
id
,answer_list
, etc., are common variables that might not clearly indicate their purpose without additional context. -
Documentation Consistency: Ensure consistency in documentation throughout the function if applicable (e.g., using consistent naming conventions).
Here is an optimized version with minor changes:
from fastapi.openapi.models import (
Responses,
Response,
Schema
)
class OpenAIChatApi(ApiMixin):
@staticmethod
def get_response_body_api() -> Responses:
"""Return OpenAPI response body definition."""
return Responses(
responses={
200: Response(
description="Response parameters",
content=Schema(
type=dict,
properties={
'conversation_id': Schema(
type=str,
title='Conversation ID'
),
'choices': Schema(
type=list,
items=Schema(
type=dict,
required=[
'message', 'finish_reason', 'index',
'answer_list']
),
properties={
'finish_reason': Schema(
type=str
),
'index': Schema(
type=int
),
'answer_list': Schema(
type=list,
items=Schema(
type=dict,
required=[
'content', 'view_type',
'runtime_node_id', 'chat_record_id',
'reasoning_content']
),
properties={
'content': Schema(
type=str
),
'view_type': Schema(
type=str
),
'runtime_node_id': Schema(
type=str
),
'chat_record_id': Schema(
type=str
),
'reasoning_content': Schema(
type=str
)
}
),
'message': Schema(
type=dict,
required=['content', 'role'],
properties={
'content': Schema(
type=str
),
'role': Schema(
type=str
)
}
),
'created': Schema(
type=int
),
'model': Schema(
type=str
),
'object': Schema(
type=str
),
'usage': Schema(
type=dict,
required=[
'completion_tokens', 'prompt_tokens', 'total_tokens'
],
properties={
'completion_tokens': Schema(
type=int
),
'prompt_tokens': Schema(
type=int
),
'total_tokens': Schema(
type=int
)
})
})
)
}
))
}
@staticmethod
def get_request_body_api() -> Schema:
return Schema(
type=dict,
properties={
# Define request payload here
}
)
These changes should make the code cleaner and potentially easier to maintain. If there are specific parts of the function or other areas where further optimization or clarification might be needed, feel free to let me know!
@@ -320,7 +320,7 @@ def one_meta(self, with_valid=False): | |||
raise AppApiException(500, _('Model does not exist')) | |||
if model.permission_type == 'PRIVATE' and str(model.user_id) != str(self.data.get("user_id")): | |||
raise Exception(_('No permission to use this model') + f"{model.name}") | |||
model = QuerySet(Model).get(id=self.data.get('id'), user_id=self.data.get('user_id')) | |||
model = QuerySet(Model).get(id=self.data.get('id')) | |||
return {'id': str(model.id), 'provider': model.provider, 'name': model.name, 'model_type': model.model_type, | |||
'model_name': model.model_name, | |||
'status': model.status, |
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 appears mostly correct, but I have some minor suggestions for improvement:
-
Variable Naming: Use consistent variable names throughout the function. The current use of
_
for temporary variables can be confusing. Consistent naming (e.g.,user
) would help maintain readability. -
Docstring and Comments: Add more detailed comments in the docstring to explain the purpose of each section of the code and provide a step-by-step flow control comment like
# Check if model exists
. -
Optimization:
- Ensure that all queries are necessary. If you don't need certain columns or fields from the database, fetching only what's needed might improve performance.
- Consider using Django's ORM caching capabilities to reduce unnecessary database hits on repeated requests.
Here's an improved version of the code with these considerations:
def one_meta(self, with_valid=False):
# Step 1: Validate input data integrity
id = self.data.get('id')
user_id = self.data.get('user_id')
if not id or not user_id:
raise AppApiException(
400, _("ID and User ID must be provided")
)
try:
# Step 2: Retrieve model based on given ID
model = QuerySet(Model).get(id=id)
# Step 3: Check permission
if model.permission_type == 'PRIVATE':
if str(user_id) != str(model.user_id):
raise Exception(_('No permission to use this model') + f" {model.name}")
# Return model metadata
return {
'id': str(model.id),
'provider': model.provider,
'name': model.name,
'model_type': model.model_type,
'model_name': model.model_name,
'status': model.status,
}
except Model.DoesNotExist:
raise AppApiException(500, _('Model does not exist'))
Make sure to adjust variable names and other components according to your specific project requirements.
@@ -37,6 +37,7 @@ class Openai(APIView): | |||
@swagger_auto_schema(operation_summary=_("OpenAI Interface Dialogue"), | |||
operation_id=_("OpenAI Interface Dialogue"), | |||
request_body=OpenAIChatApi.get_request_body_api(), | |||
responses=OpenAIChatApi.get_response_body_api(), | |||
tags=[_("OpenAI Dialogue")]) | |||
def post(self, request: Request, application_id: str): | |||
return OpenAIChatSerializer(data={'application_id': application_id, 'client_id': request.auth.client_id, |
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 appears to be correct and does not contain any irregularities, potential issues, or optimization suggestions. It defines an API view withSwagger documentation that outlines the post
method's operation summary, details, request body schema, response body schema, and applicable tags. The implementation also correctly assigns values from the request object (request.auth.client_id
) into the serializer data dictionary. There is no apparent room for improvement in terms of efficiency or readability for this particular snippet.
feat: Swagger document response for adding OpenAI interface