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

Added option to override GrpcChannelOptions when adding DaprWorkflow (#7218) #1244

Merged
merged 1 commit into from
Nov 4, 2024
Merged

Added option to override GrpcChannelOptions when adding DaprWorkflow (#7218) #1244

merged 1 commit into from
Nov 4, 2024

Conversation

humandigital-ruud
Copy link
Contributor

@humandigital-ruud humandigital-ruud commented Feb 23, 2024 •
edited by WhitWaldo
Loading

Description

You can now use custom GrpcChannelOptions when using DaprWorkflow, in the same manner as was already the case when creating the DaprClient:

builder.Services.AddDaprWorkflow(options =>
{
    options.UseGrpcChannelOptions(new GrpcChannelOptions
    {
        MaxReceiveMessageSize = 32 * 1024 * 1024,
        MaxSendMessageSize = 32 * 1024 * 1024
    });
});

Issue reference

See issue: GRPC data receive limits (#7218)
Fixes: #1163

philliphoff
philliphoff previously requested changes Feb 27, 2024
View reviewed changes
Copy link
Collaborator

@philliphoff philliphoff left a 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 humandigital-ruud mentioned this pull request Feb 28, 2024
Open
@philliphoff
Copy link
Collaborator

@humandigital-ruud Are you still interested in pursuing this PR?

@humandigital-ruud
Copy link
Contributor Author

@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

@humandigital-michiel
Copy link
Contributor

@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!

@humandigital-michiel
Copy link
Contributor

humandigital-michiel commented Jul 18, 2024 •
edited
Loading

hi @philliphoff i made some adjustments based on your feedback. One of my first contributions to dapr.

@philliphoff gentle reminder for this one.

@olitomlinson olitomlinson mentioned this pull request Sep 24, 2024
Open
60 tasks
@humandigital-michiel
Copy link
Contributor

Refactored the changes.
We need to do some internal review, because fundamentals changed.
There is no unit test anymore on Workflow, because these we're removed some time ago, do you want these back?

WhitWaldo
WhitWaldo approved these changes Oct 30, 2024
View reviewed changes
Copy link
Contributor

@WhitWaldo WhitWaldo 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!

@WhitWaldo WhitWaldo dismissed philliphoff’s stale review October 30, 2024 03:35

Review created prior to refactoring

@WhitWaldo WhitWaldo added this to the v1.15 milestone Oct 30, 2024
@WhitWaldo
Copy link
Contributor

@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!

@humandigital-michiel
Refactor DaprWorkflowClientBuilderFactory and WorkflowRuntimeOptions
3948d5b 
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 <[email protected]>
@WhitWaldo WhitWaldo merged commit e7d3c47 into dapr:master Nov 4, 2024
10 checks passed
@WhitWaldo
Copy link
Contributor

Thank you for your contribution @humandigital-michiel and @humandigital-ruud !

@WhitWaldo WhitWaldo mentioned this pull request Nov 5, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@humandigital-michiel humandigital-michiel humandigital-michiel left review comments

@WhitWaldo WhitWaldo WhitWaldo approved these changes

@philliphoff philliphoff philliphoff left review comments

Assignees

@humandigital-ruud humandigital-ruud

Labels
area/workflow
Projects
None yet
Milestone
v1.15
Development

Successfully merging this pull request may close these issues.

Enable specifying Grpc options like " MaxReceiveMessageSize" in WorkflowServiceCollectionExtensions
4 participants
@humandigital-ruud @philliphoff @humandigital-michiel @WhitWaldo