[WIP][event-hubs] EventHubBufferedProducerClient implementation #18106
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…dle connection timeout watchers
…ressure to EventHubBufferedProducerClient
chradek
requested review from
bterlson,
HarshaNalluru,
richardpark-msft and
xirzec
as code owners
deyaaeldeen
reviewed
Oct 14, 2021
Comment on lines
+109
to
+111
| if (this.getCurrentBufferedCount() === 0) { | ||
| return Promise.resolve(); | ||
| } |
There was a problem hiding this comment.
NIT: this seems to be the base case so I would prefer to move it up for readability.
deyaaeldeen
reviewed
Oct 14, 2021
Comment on lines
+210
to
+213
| } else if (!didAdd) { | ||
| // Handles the case where the event _was_ successfull added to the new batch. | ||
| this._batchedEvents.push(event); | ||
| } |
There was a problem hiding this comment.
I do not understand this case. If it was not added to batch, why is it being added to _batchedEvents here?
deyaaeldeen
approved these changes
Oct 14, 2021
6 tasks
deyaaeldeen
added a commit
that referenced
this pull request
Nov 13, 2021
* [event-hubs] EventHubsBufferedProducerClient skeleton (#17761) * temp transfer * [event-hubs] initial EventHubBufferedProducerClient skeleton * Change default value of BufferedCloseOptions.flush to true instead of false * Reorder doc comment content for EventHubBufferedProducerClient * remove eslint exception for @azure/azure-sdk/ts-naming-options in EventHubBufferedProducerClientOptions * add enqueueEvent (singular) method * update API view * [WIP][event-hubs] EventHubBufferedProducerClient implementation (#18106) * transfer * [mock-hub] fix issue where calling stop() would not remove existing idle connection timeout watchers * [event-hubs] reduce repetition in hubruntime.spec.ts tests * Add support for flush,eventSuccess and eventError handlers, and backpressure to EventHubBufferedProducerClient * add documentation and update API to align with .NET * [Buffered Event Hubs Producer] Implements Parition Key to Partition ID mapping (#18331) * [Buffered Event Hubs Producer] Implements Parition Key to Partition ID mapping * fix the implementation * factor out the hashing logic * remove unused import * the test pass but the implementation needs to be simplified * simplified implementation * address feedback * address feedback * fix format * address feedback Co-authored-by: chradek <[email protected]>
cochi2
added a commit
that referenced
this pull request
Nov 15, 2021
* Removing return response for Pause/Resume/Stop recording * Removing validations * Fixes after merge * Added downloadToFile operation * Added downloadToFile test * Updating API MD file * Fixing format auto * Renaming files * [KeyVault] - Move MHSM to resource group location (#18664) Moving the Managed HSM to the same location as our resource group now that the limits have been expanded. * Add default cloud configuration values to source (#18653) Co-authored-by: Ben Broderick Phillips <[email protected]> * simplify the commit history so that the patch can apply properly (#18665) Co-authored-by: scbedd <[email protected]> * Updating name for header * [Event Hubs] Merge feature branch for buffered producer (#18590) * [event-hubs] EventHubsBufferedProducerClient skeleton (#17761) * temp transfer * [event-hubs] initial EventHubBufferedProducerClient skeleton * Change default value of BufferedCloseOptions.flush to true instead of false * Reorder doc comment content for EventHubBufferedProducerClient * remove eslint exception for @azure/azure-sdk/ts-naming-options in EventHubBufferedProducerClientOptions * add enqueueEvent (singular) method * update API view * [WIP][event-hubs] EventHubBufferedProducerClient implementation (#18106) * transfer * [mock-hub] fix issue where calling stop() would not remove existing idle connection timeout watchers * [event-hubs] reduce repetition in hubruntime.spec.ts tests * Add support for flush,eventSuccess and eventError handlers, and backpressure to EventHubBufferedProducerClient * add documentation and update API to align with .NET * [Buffered Event Hubs Producer] Implements Parition Key to Partition ID mapping (#18331) * [Buffered Event Hubs Producer] Implements Parition Key to Partition ID mapping * fix the implementation * factor out the hashing logic * remove unused import * the test pass but the implementation needs to be simplified * simplified implementation * address feedback * address feedback * fix format * address feedback Co-authored-by: chradek <[email protected]> * [Event Hubs] Prepare release (#18672) * [Event Hubs] Prepare release * remove empty sections * Node doesn't support some samples for smoke test (#18496) * Node doesn't support some samples for smoke test * [search-documents] reprocessed samples with exp. generator Co-authored-by: Will Temple <[email protected]> * Add rlc quickstart guideline (#18503) * add llc quickstart guideline * update format * add documentation about ci.yml * update to resolve some comments * update to resolve some comments * update to resolve comments * updates term * update format * update format * Running Rush update * Sync eng/common directory with azure-sdk-tools for PR 2265 (#18683) * Improve devops logging for link checker * Update eng/common/scripts/Verify-Links.ps1 Co-authored-by: Wes Haggard <[email protected]> Co-authored-by: Wes Haggard <[email protected]> * Build and test predefined set of packages only in identity pipelines (#18686) * Build and test predefined set of packages only in identity pipelines * Fixing playback testing * Run rushx format Co-authored-by: Maor Leger <[email protected]> Co-authored-by: Azure SDK Bot <[email protected]> Co-authored-by: Ben Broderick Phillips <[email protected]> Co-authored-by: scbedd <[email protected]> Co-authored-by: Deyaaeldeen Almahallawi <[email protected]> Co-authored-by: chradek <[email protected]> Co-authored-by: Sarangan Rajamanickam <[email protected]> Co-authored-by: Will Temple <[email protected]> Co-authored-by: Qiaoqiao Zhang <[email protected]> Co-authored-by: Wes Haggard <[email protected]> Co-authored-by: Wes Haggard <[email protected]> Co-authored-by: praveenkuttappan <[email protected]>
zihzhan-msft
pushed a commit
that referenced
this pull request
Nov 16, 2021
* [KeyVault] - Move MHSM to resource group location (#18664) Moving the Managed HSM to the same location as our resource group now that the limits have been expanded. * Add default cloud configuration values to source (#18653) Co-authored-by: Ben Broderick Phillips <[email protected]> * simplify the commit history so that the patch can apply properly (#18665) Co-authored-by: scbedd <[email protected]> * [Event Hubs] Merge feature branch for buffered producer (#18590) * [event-hubs] EventHubsBufferedProducerClient skeleton (#17761) * temp transfer * [event-hubs] initial EventHubBufferedProducerClient skeleton * Change default value of BufferedCloseOptions.flush to true instead of false * Reorder doc comment content for EventHubBufferedProducerClient * remove eslint exception for @azure/azure-sdk/ts-naming-options in EventHubBufferedProducerClientOptions * add enqueueEvent (singular) method * update API view * [WIP][event-hubs] EventHubBufferedProducerClient implementation (#18106) * transfer * [mock-hub] fix issue where calling stop() would not remove existing idle connection timeout watchers * [event-hubs] reduce repetition in hubruntime.spec.ts tests * Add support for flush,eventSuccess and eventError handlers, and backpressure to EventHubBufferedProducerClient * add documentation and update API to align with .NET * [Buffered Event Hubs Producer] Implements Parition Key to Partition ID mapping (#18331) * [Buffered Event Hubs Producer] Implements Parition Key to Partition ID mapping * fix the implementation * factor out the hashing logic * remove unused import * the test pass but the implementation needs to be simplified * simplified implementation * address feedback * address feedback * fix format * address feedback Co-authored-by: chradek <[email protected]> * [Event Hubs] Prepare release (#18672) * [Event Hubs] Prepare release * remove empty sections * Node doesn't support some samples for smoke test (#18496) * Node doesn't support some samples for smoke test * [search-documents] reprocessed samples with exp. generator Co-authored-by: Will Temple <[email protected]> * Add rlc quickstart guideline (#18503) * add llc quickstart guideline * update format * add documentation about ci.yml * update to resolve some comments * update to resolve some comments * update to resolve comments * updates term * update format * update format * Sync eng/common directory with azure-sdk-tools for PR 2265 (#18683) * Improve devops logging for link checker * Update eng/common/scripts/Verify-Links.ps1 Co-authored-by: Wes Haggard <[email protected]> Co-authored-by: Wes Haggard <[email protected]> * Build and test predefined set of packages only in identity pipelines (#18686) * Build and test predefined set of packages only in identity pipelines * Removed raw response from ContentDownloadResponse * Changing name to RepeatanleContentDownloadResult * Changed name to ContentDownloadResult * Fixes after merging Co-authored-by: Maor Leger <[email protected]> Co-authored-by: Azure SDK Bot <[email protected]> Co-authored-by: Ben Broderick Phillips <[email protected]> Co-authored-by: scbedd <[email protected]> Co-authored-by: Deyaaeldeen Almahallawi <[email protected]> Co-authored-by: chradek <[email protected]> Co-authored-by: Sarangan Rajamanickam <[email protected]> Co-authored-by: Will Temple <[email protected]> Co-authored-by: Qiaoqiao Zhang <[email protected]> Co-authored-by: Wes Haggard <[email protected]> Co-authored-by: Wes Haggard <[email protected]> Co-authored-by: praveenkuttappan <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently a WIP.
Status report:
EventHubBufferedProducerClient.enqueueEvent()EventHubBufferedProducerClient.enqueueEvents()EventHubBufferedProducerClient.flush()EventHubBufferedProducerClient.close()with flush flag.EventHubBufferedProducerClient.enqueueEvent[s]()when buffer is full.onSendEventsSuccessHandlerwhen events are successfully sent to service.onSendEventsErrorHandlerwhen events error when sending to service.enqueuecalls.abortSignaloptimizations - all async methods should respect theabortSignalSome tests have been written to test back-pressure and enqueueEvent.