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

increase grpc payload limit and JSON payload limit #285

Merged
merged 2 commits into from
Dec 19, 2024

Conversation

dinmukhamedm
Copy link
Member

@dinmukhamedm dinmukhamedm commented Dec 19, 2024

Important

Increase HTTP and gRPC payload limits to 100MB in main.rs.

  • Payload Limits:
    • Increase HTTP_PAYLOAD_LIMIT to 100MB in main.rs.
    • Increase GRPC_PAYLOAD_DECODING_LIMIT to 100MB in main.rs.
  • Configuration:
    • Apply PayloadConfig and JsonConfig with HTTP_PAYLOAD_LIMIT in the /v1 scope.
    • Set max_decoding_message_size for gRPC service with GRPC_PAYLOAD_DECODING_LIMIT.

This description was created by Ellipsis for b6b7fbb. It will automatically update as commits are pushed.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Looks good to me! Reviewed everything up to 1835356 in 1 minute and 31 seconds

More details
  • Looked at 52 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. app-server/src/main.rs:400
  • Draft comment:
    The previous PayloadConfig for the semantic_search service was removed. Ensure that the new HTTP_PAYLOAD_LIMIT is sufficient for all services, including semantic_search.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The PayloadConfig for the HTTP server was updated to 100MB, but the previous configuration for the semantic_search service was removed. This might be an oversight.

Workflow ID: wflow_cKSM4MvnlsIMegog


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Looks good to me! Incremental review on b6b7fbb in 10 seconds

More details
  • Looked at 15 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. app-server/src/main.rs:78
  • Draft comment:
    Ensure that all functions and configurations using HTTP_PAYLOAD_LIMIT and GRPC_PAYLOAD_DECODING_LIMIT are compatible with usize. The change from u64 to usize is appropriate for memory-related operations.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The change from u64 to usize for payload limits is appropriate for memory-related operations, as usize is architecture-dependent and suitable for indexing and memory size operations. However, it's important to ensure that all related functions and configurations that use these constants are compatible with usize.

Workflow ID: wflow_VxDTxKOcaGjrAWlY


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@skull8888888 skull8888888 merged commit 5ed8d4f into dev Dec 19, 2024
1 check passed
@skull8888888 skull8888888 deleted the fix/payload-size branch December 19, 2024 19:46
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