-
Notifications
You must be signed in to change notification settings - Fork 335
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
Added option to override GrpcChannelOptions when adding DaprWorkflow (#7218) #1244
base: master
Are you sure you want to change the base?
Conversation
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.
A couple of comments about how gRPC options are exposed to both workflow runtime and client callers. Also, it'd be good to have some unit/integration tests built around at least the main scenario this is intended to solve (i.e. increasing the max. message size).
@humandigital-ruud Are you still interested in pursuing this PR? |
I will discuss this with my co-worker to see if he can take this over from me and spend some more time on it to get it right |
Hi! |
4df8853
to
777a11b
Compare
hi @philliphoff i made some adjustments based on your feedback. One of my first contributions to dapr. @philliphoff gentle reminder for this one. |
777a11b
to
b3a368a
Compare
d9bba92
to
ee8be67
Compare
This commit refactors the DaprWorkflowClientBuilderFactory and WorkflowRuntimeOptions classes. In DaprWorkflowClientBuilderFactory: - Added a new method, UseGrpcChannelOptions, to allow the use of custom GrpcChannelOptions for creating the GrpcChannel. - Updated the UseGrpc method to use the GrpcChannelOptions provided by the WorkflowRuntimeOptions. In WorkflowRuntimeOptions: - Added a new property, GrpcChannelOptions, to store the custom GrpcChannelOptions. - Added a new method, UseGrpcChannelOptions, to set the GrpcChannelOptions. These changes improve the flexibility and customization options for the Dapr workflow client. Signed-off-by: Michiel van Praat <michiel.vanpraat@humandigital.nl>
Refactored the changes. |
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.
Looks good to me!
Review created prior to refactoring
@humandigital-michiel This looks good on my end, but I'm showing the branch is out of date. Could you please merge the latest from master so I can complete this PR? Thanks for putting this together! |
Description
You can now use custom GrpcChannelOptions when using DaprWorkflow, in the same manner as was already the case when creating the DaprClient:
Issue reference
See issue: GRPC data receive limits (#7218)