Skip to content

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

Merged
merged 1 commit into from
Apr 2, 2025

Conversation

shaohuzhang1
Copy link
Contributor

feat: Swagger document response for adding OpenAI interface

Copy link

f2c-ci-robot bot commented Apr 2, 2025

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 Apr 2, 2025

[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

@shaohuzhang1 shaohuzhang1 merged commit add1cba into main Apr 2, 2025
4 checks passed
@shaohuzhang1 shaohuzhang1 deleted the pr@main@feat_openai_response branch April 2, 2025 11:59
})

}))})

@staticmethod
def get_request_body_api():
return openapi.Schema(type=openapi.TYPE_OBJECT,
Copy link
Contributor Author

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:

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

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

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

  4. 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,
Copy link
Contributor Author

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:

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

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

  3. 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,
Copy link
Contributor Author

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.

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.

1 participant