Skip to content
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: Import and Export function lib #2294

Merged
merged 1 commit into from
Feb 17, 2025
Merged

Conversation

shaohuzhang1
Copy link
Contributor

feat: Import and Export function lib

@has_permissions(RoleConstants.ADMIN, RoleConstants.USER)
def get(self, request: Request, id: str):
return FunctionLibSerializer.Operate(
data={'id': id, 'user_id': request.user.id}).export() No newline at end of file
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 Python code is mostly clear and well-documented, but there are a few areas that can be optimized or improved:

Optimizations/Recommendations

  1. Avoid Unnecessary Imports:

    • The group import (from common.constants.permission_constants import Group) and operate import (from common.constants.permission_constants import Operate) at the beginning seem unnecessary since they are only used within specific functions, such as Import, which might make it more efficient to import them locally when needed.
  2. Simplify Function Descriptions:

    • In FunctionLibApi.Import.get_request_params_api() and FunctionLibApi.Export.get_request_params_api(), consider using simpler descriptions without including long strings like _("Import function") multiple times.
  3. Use Context Managers Properly:

    • Ensure that you use context managers properly, especially if handling files (e.g., request.FILES.get('file')). This helps manage resources efficiently and avoids potential resource leaks.
  4. Consistent Error Handling:

    • Consider adding consistent error handling across different parts of the viewset. If an exception occurs during file processing in Import.post(), ensure that appropriate responses are returned.
  5. Docstring Clarity:

    • Improve clarity and completeness of docstrings for methods that handle requests, ensuring all parameters and return values are documented accurately.
  6. Performance Considerations:

    • If this viewset handles large datasets or frequent operations, consider implementing caching strategies to improve performance.

Here's a refined version of the Import method with some optimizations:

class Import(APIView):
    authentication_classes = [TokenAuth]
    parser_classes = [MultiPartParser]

    @action(methods="POST", detail=False)
    @swagger_auto_schema(operation_summary=_("Import function"), operation_id=_("Import function"),
                         manual_parameters=[
                             FunctionLibApi.Import.get_request_params_api().get("file")
                         ],
                         tags=[_("function")]
                         )
    def post(self, request: Request):
        try:
            # Read the uploaded file content
            file_content = request.FILES['file'].read()

            return result.success(
                FunctionLibSerializer.Import(data={
                    'user_id': request.user.id,
                    'data': file_content  # Assuming imported data comes from a field named "data"
                }).import_()
            
        except FileNotFoundError:
            return result.failure(_("File not found"), status_code=404)
        
        except Exception as e:
            return result.failure(str(e), log_exception=True)  # Log exceptions for debugging purposes

This refactored version reduces redundancy by importing necessary elements locally and improves error handling by explicitly catching and responding to file-related errors.

@liuruibin liuruibin merged commit f45855c into main Feb 17, 2025
4 checks passed
@liuruibin liuruibin deleted the pr@main@feat_support_import_fun branch February 17, 2025 02:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants