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

[event-hubs] update sendBatch to accept lists of events #8622

Merged
merged 2 commits into from
Apr 30, 2020

Conversation

chradek
Copy link
Contributor

@chradek chradek commented Apr 30, 2020

Fixes #8470

Updates the EventHubProducerClient.sendBatch method to also accept an array of EventDataBatch. This also necessitated adding a new options type that let's the user specify a partitionKey or partitionId.

Because of the additional options, I also needed to add validation to ensure that none of the options conflicted with the options that can be set on a batch in the createBatch call.

@chradek chradek added Client This issue points to a problem in the data-plane of the library. Event Hubs labels Apr 30, 2020
* events are sent to the associated Event Hub.
* - `abortSignal` : A signal the request to cancel the send operation.
* - `partitionId` : The partition this batch will be sent to. If set, `partitionKey` can not be set.
* - `partitionKey` : A value that is hashed to produce a partition assignment.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* - `partitionKey` : A value that is hashed to produce a partition assignment.
* - `partitionKey` : A value that is hashed to produce a partition assignment. If set, `partitionId` can not be set.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated, thank you!

* Options to configure the `sendBatch` method on the `EventHubProducerClient`.
* Options to configure the `sendBatch` method on the `EventHubProducerClient`
* when sending an array of events.
* If `partitionId` is set, `partitionKey` must not be set.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* If `partitionId` is set, `partitionKey` must not be set.
* If `partitionId` is set, `partitionKey` must not be set and vice versa.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated, thank you!

Copy link
Contributor

@ramya-rao-a ramya-rao-a left a comment

Choose a reason for hiding this comment

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

In Service Bus, we are re-using the batch class and using tryAdd when sending array of messages.

I prefer the approach here because we get to skip being in the business of forming the error message when batch is too big, we rely on the service instead.
On the other hand, there is a lot of duplicated code lying around in this approach.

For now, lets go ahead with what we have here.

@chradek chradek merged commit 9afbdaa into Azure:master Apr 30, 2020
@chradek chradek deleted the eh-send-batch-array branch April 30, 2020 16:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Client This issue points to a problem in the data-plane of the library. Event Hubs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants