-
Notifications
You must be signed in to change notification settings - Fork 361
- Added a new method PublishBulkByteEventAsync
in DaprClientGrpc.cs
#1638
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
base: master
Are you sure you want to change the base?
- Added a new method PublishBulkByteEventAsync
in DaprClientGrpc.cs
#1638
Conversation
to enable publishing multiple messages in a single call via gRPC. - Method accepts IEnumerable<byte[]> and publishes them to a specified pub/sub topic, improving performance over repeated PublishEventAsync calls. - This change aligns the .NET SDK with the bulk publish pattern in the Go SDK. - Ensured backward compatibility; existing PublishEventAsync remains unchanged. - Updated internal gRPC serialization and invocation logic to handle batch messages. - Tested locally by publishing bulk messages to Redis pub/sub through a producer project. This commit is part of the feature branch `feature/PublishBulkByteEventAsync`.
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.
Good catch - yes, that's certainly an oversight.
In the future, please avoid PRs with a bunch of minor formatting changes. As I explained in one of the comments, those tend to be styled based on your own personal IDE preferences and settings. It just distracts from the actual code you're submitting and will just get flagged in all other contributors' IDEs as opportunities to fix when I'd really rather not get into formatting specifications.
string key, | ||
ConsistencyMode? consistencyMode = null, | ||
IReadOnlyDictionary<string, string>? metadata = null, | ||
string storeName, |
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.
Generally, please don't submit PRs with formatting changes like this as everyone's IDE is going to add/remove stuff like this based on local preferences and it slows down actually reviewing the changes you've made.
string pubsubName, | ||
string topicName, | ||
IReadOnlyList<ReadOnlyMemory<byte>> events, | ||
string dataContentType = "application/json", |
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.
For consistency, could you please have this use the same referenced constant as the non-bulk equivalent, Constants.ContentTypeApplicationJson
?
string pubsubName, | ||
string topicName, | ||
IReadOnlyList<ReadOnlyMemory<byte>> events, | ||
string dataContentType = "application/json", |
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.
Again, please update the default value to use the pre-existing constant Constants.ContentTypeApplicationJson
.
Dictionary<string, string>? metadata = null, | ||
CancellationToken cancellationToken = default) | ||
{ | ||
var request = new Autogenerated.BulkPublishRequest |
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.
Is there a reason we cannot refactor and use the existing generic MakeBulkPublishRequest
here instead of duplicating the method for this additional type?
var envelope = new Autogenerated.InvokeServiceRequest() | ||
{ | ||
Id = appId, Message = new Autogenerated.InvokeRequest() { Method = methodName, }, | ||
Id = appId, |
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.
To this and the remaining formatting changes - same as above, please don't submit these generally.
public async Task PublishBulkByteEventAsync_PublishesEventsAndReturnsResponse() | ||
{ | ||
// Arrange | ||
var client = new DaprClientBuilder().Build(); |
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.
Unit tests aren't run with a running instance of Dapr, so this isn't going to work correctly. I would advise copying the example above using the TestClient
it produces so you can monitor the "captured" gRPC request and validate against that payload.
to enable publishing multiple messages in a single call via gRPC.
This commit is part of the feature branch
feature/PublishBulkByteEventAsync
.Description
1️⃣ New Method: PublishBulkByteEventAsync
Where added: DaprClient.cs (abstract method), DaprClientGrpc.cs (gRPC implementation), DaprClientHttp.cs (HTTP implementation), and IDaprClient.cs (interface).
Purpose: Allow publishing multiple raw byte[] messages in a single request to a Dapr pub/sub component.
Reason: The official .NET SDK did not have a built-in bulk byte publish method like the Go SDK’s PublishBulkEvent.
Code Example
public abstract Task PublishBulkByteEventAsync(
string pubsubName,
string topicName,
IEnumerable<byte[]> events,
Dictionary<string, string>? metadata = null,
CancellationToken cancellationToken = default);
2️⃣ gRPC Implementation (DaprClientGrpc.cs)
Converts each byte[] event into a BulkPublishRequestEntry using ByteString.CopyFrom.
Adds optional metadata.
Calls the gRPC PublishBulkEventAsync API of the Dapr sidecar.
Why
Mirrors the Go SDK’s PublishBulkEvent implementation.
Ensures high-performance bulk publish without serializing to JSON unnecessarily.
3️⃣ HTTP Implementation (DaprClientHttp.cs)
Uses the Dapr HTTP bulk publish API:
POST /v1.0-alpha1/publish/bulk/{pubsub}/{topic}
Each event is Base64-encoded for transport (Convert.ToBase64String(e)) and labeled with application/octet-stream.
Sends all events in a single HTTP request.
Why
Supports environments where gRPC is not available.
Maintains parity with the Go SDK bulk publish behavior.
Issue reference
We strive to have all PR being opened based on an issue, where the problem or feature have been discussed prior to implementation.
Please reference the issue this PR will close: #[issue number]
Checklist
Please make sure you've completed the relevant tasks for this PR, out of the following list: