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.

Sign up for GitHub

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

Jump to bottom

feat: make max message size configurable for gRPC clients #983

Merged
merged 1 commit into from
Feb 18, 2024
Merged

feat: make max message size configurable for gRPC clients #983

merged 1 commit into from
Feb 18, 2024

Conversation

etolbakov
Copy link
Contributor

Which issue does this PR close?

Closes #930
Relates to #931

Rationale for this change

Make max message size configurable for gRPC clients.

What changes are included in this PR?

Updated BallistaConfig code and adjusted tests

Are there any user-facing changes?

no

@etolbakov
Copy link
Contributor Author

@andygrove
Could you please take a look when you have time?

Two main questions that I have:

  1. the naming convention for the config keys (should it be . or _ separated). Please let me know if any adjustments should be made.
  2. failed to find if distributed_query.rs, executor_process.rs, executor_server.rs are using BallistaConfig in order to apply the same change that I did for context.rs. Does the scope of this PR is suppose to add config as well?

andygrove
andygrove approved these changes Feb 18, 2024
View reviewed changes
Copy link
Member

@andygrove andygrove left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks @etolbakov!

@andygrove
Copy link
Member

I started looking at your question 2 and I think it will be easiest for me to put up a PR to explain. I should have that up shortly.

@andygrove andygrove merged commit f7da222 into apache:main Feb 18, 2024
16 checks passed
@etolbakov etolbakov deleted the etolbakov/grpc-message-size-configurable branch February 18, 2024 18:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andygrove andygrove andygrove approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Make max message size configurable in gRPC clients
2 participants
@etolbakov @andygrove