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

[Event Hubs] Merge feature branch for buffered producer #18590

Merged
merged 6 commits into from
Nov 13, 2021
Merged

[Event Hubs] Merge feature branch for buffered producer #18590

merged 6 commits into from
Nov 13, 2021

Conversation

deyaaeldeen
Copy link
Member

@deyaaeldeen deyaaeldeen commented Nov 9, 2021 •
edited
Loading

Merging the feature branch into main.

Fixes #17699

chradek and others added 3 commits October 7, 2021 08:06
@chradek
[event-hubs] EventHubsBufferedProducerClient skeleton (#17761)
14105b2 
* 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
@chradek
[WIP][event-hubs] EventHubBufferedProducerClient implementation (#18106)
339bc99 
* 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
@deyaaeldeen
[Buffered Event Hubs Producer] Implements Parition Key to Partition I…
736bdff 
…D 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
@ghost ghost added the Event Hubs label Nov 9, 2021
@azure-sdk
Copy link
Collaborator

API changes have been detected in @azure/event-hubs. You can review API changes here

@deyaaeldeen deyaaeldeen marked this pull request as ready for review November 9, 2021 16:51
@deyaaeldeen deyaaeldeen changed the title [Event Hubs] Merge feature branch [Event Hubs] Merge feature branch for buffered producer Nov 9, 2021
jsquire
jsquire reviewed Nov 9, 2021
View reviewed changes
sdk/eventhub/event-hubs/src/batchingPartitionChannel.ts Outdated Show resolved Hide resolved
sdk/eventhub/event-hubs/src/impl/partitionAssigner.ts Show resolved Hide resolved
sdk/eventhub/event-hubs/src/eventHubBufferedProducerClient.ts Show resolved Hide resolved
sdk/eventhub/event-hubs/src/eventHubBufferedProducerClient.ts
/**
* Returns the total number of buffered events across all partitions.
*/
private _getTotalBufferedEventsCount(): number {
Copy link
Member

Choose a reason for hiding this comment

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

This seems like it may be an expensive call to execute after each enqueue operation. Should we consider tracking the value separately to allow for a more efficient read?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point, and to clarify, are you proposing adding a new API to return the events count in the buffer and make queueEvent return void?

Copy link
Member Author

Choose a reason for hiding this comment

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

We can take this up in beta.2 after I get more information on this.

sdk/eventhub/event-hubs/src/eventHubBufferedProducerClient.ts Outdated Show resolved Hide resolved
sdk/eventhub/event-hubs/src/eventHubBufferedProducerClient.ts Show resolved Hide resolved
sdk/eventhub/event-hubs/src/eventHubBufferedProducerClient.ts Show resolved Hide resolved
@deyaaeldeen deyaaeldeen mentioned this pull request Nov 12, 2021
Merged
jeremymeng
jeremymeng approved these changes Nov 13, 2021
View reviewed changes
Copy link
Contributor

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

sdk/eventhub/event-hubs/review/event-hubs.api.md Outdated Show resolved Hide resolved
@deyaaeldeen deyaaeldeen enabled auto-merge (squash) November 13, 2021 00:20
@azure-sdk
Copy link
Collaborator

API changes have been detected in @azure/event-hubs. You can review API changes here

@deyaaeldeen deyaaeldeen merged commit 8d28332 into main Nov 13, 2021
@deyaaeldeen deyaaeldeen deleted the feature/eh-buffered-producer branch November 13, 2021 00:46
cochi2 added a commit that referenced this pull request Nov 15, 2021
@cochi2 @maorleger
@azure-sdk @benbp @scbedd @deyaaeldeen @chradek @sarangan12 @willmtemple @qiaozha @weshaggard @praveenkuttappan
Applying comments from API Review (#18662)
68337e8 
* 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 <bebroder@microsoft.com>

* simplify the commit history so that the patch can apply properly (#18665)

Co-authored-by: scbedd <45376673+scbedd@users.noreply.github.com>

* 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 <51000525+chradek@users.noreply.github.com>

* [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 <will@wtemple.net>

* 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 <Wes.Haggard@microsoft.com>
Co-authored-by: Wes Haggard <weshaggard@users.noreply.github.com>

* 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 <maorleger@users.noreply.github.com>
Co-authored-by: Azure SDK Bot <53356347+azure-sdk@users.noreply.github.com>
Co-authored-by: Ben Broderick Phillips <bebroder@microsoft.com>
Co-authored-by: scbedd <45376673+scbedd@users.noreply.github.com>
Co-authored-by: Deyaaeldeen Almahallawi <dealmaha@microsoft.com>
Co-authored-by: chradek <51000525+chradek@users.noreply.github.com>
Co-authored-by: Sarangan Rajamanickam <sarajama@microsoft.com>
Co-authored-by: Will Temple <will@wtemple.net>
Co-authored-by: Qiaoqiao Zhang <55688292+qiaozha@users.noreply.github.com>
Co-authored-by: Wes Haggard <Wes.Haggard@microsoft.com>
Co-authored-by: Wes Haggard <weshaggard@users.noreply.github.com>
Co-authored-by: praveenkuttappan <55455725+praveenkuttappan@users.noreply.github.com>
zihzhan-msft pushed a commit that referenced this pull request Nov 16, 2021
@cochi2 @maorleger
@azure-sdk @benbp @scbedd @deyaaeldeen @chradek @sarangan12 @willmtemple @qiaozha @weshaggard @praveenkuttappan
Fixing naming for Download returning methods. (#18703)
85893a8 
* [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 <bebroder@microsoft.com>

* simplify the commit history so that the patch can apply properly (#18665)

Co-authored-by: scbedd <45376673+scbedd@users.noreply.github.com>

* [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 <51000525+chradek@users.noreply.github.com>

* [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 <will@wtemple.net>

* 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 <Wes.Haggard@microsoft.com>
Co-authored-by: Wes Haggard <weshaggard@users.noreply.github.com>

* 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 <maorleger@users.noreply.github.com>
Co-authored-by: Azure SDK Bot <53356347+azure-sdk@users.noreply.github.com>
Co-authored-by: Ben Broderick Phillips <bebroder@microsoft.com>
Co-authored-by: scbedd <45376673+scbedd@users.noreply.github.com>
Co-authored-by: Deyaaeldeen Almahallawi <dealmaha@microsoft.com>
Co-authored-by: chradek <51000525+chradek@users.noreply.github.com>
Co-authored-by: Sarangan Rajamanickam <sarajama@microsoft.com>
Co-authored-by: Will Temple <will@wtemple.net>
Co-authored-by: Qiaoqiao Zhang <55688292+qiaozha@users.noreply.github.com>
Co-authored-by: Wes Haggard <Wes.Haggard@microsoft.com>
Co-authored-by: Wes Haggard <weshaggard@users.noreply.github.com>
Co-authored-by: praveenkuttappan <55455725+praveenkuttappan@users.noreply.github.com>
azure-sdk pushed a commit to azure-sdk/azure-sdk-for-js that referenced this pull request Apr 14, 2022
@bcho @zqingqing1
@yizhang4321 @wenxuan0923 @ZeroMagic @alexeldeib
CodeGen from PR 18633 in Azure/azure-rest-api-specs
c16c82c 
Review request for Microsoft.ContainerService to add version 2022-03-02-preview (Azure#18633)

* Adds base for updating Microsoft.ContainerService from version preview/2022-02-02-preview to version 2022-03-02-preview

* Updates readme

* Updates API version in new specs and examples

* feat: add `/rotateServiceAccountSigningKeys` API (Azure#18359)

* feat: add `/rotateServiceAccountSigningKeys` API

* chore: remove duplicated description

* feat: add `workloadIdentity` settings to `SecurityProfile` (Azure#18360)

* fix: update example name (Azure#18382)

* add creationData to mc data (Azure#18414)

* add creationData to mc data

* fix test

* fix format

* Add API properties and example JSON for Web App Routing of IngressProfile (Azure#18564)

* Add API properties and example JSON for Web App Routing of IngressProfile.

* Add a ending period for description to match the style in all other "descriptions" in the same file.

* Update readmes for the 2022-03-02-preview dev branch of container service (Azure#18358)

* update readme

* update sdk readmes

* Agentpool alias minor version 2022-03-02-preview (Azure#18381)

* Add field currentOrchestratorVersion to support Agentpool alias minor version

* Add new exmaple for alias minor version

* Remove example for PrettierCheck

* Fix typo

* Latest patch version supported is 1.22.7 at the moment

* Address comments - refine descriptions for fields

* feat: add ManagedCluster StorageProfile in 0302preview (Azure#18590)

Signed-off-by: Ji An Liu <jiliu8@microsoft.com>

* Swagger change for ignore-pod-disruption-budget (Azure#18548)

* Swagger change for ignore-pod-disruption-budget

* Change ignorePodDisruptionBudget to string in example file.

* Change ignorePodDisruptionBudget to boolean type.

* add effectiveNoProxy for AKS (Azure#18544)

* add effectiveNoProxy for AKS

* add to correct api

* fix lowercase -> uppercase O

* spelling

* Replace common type definitions with references since 2022-03-02-preview for Microsoft.ContainerService  (Azure#18567)

* replace Resource

* replace SystemData

* replace parameters

* change track2 to python

Co-authored-by: hbc <bcxxxxxx@gmail.com>
Co-authored-by: Qingqing <qizhe@microsoft.com>
Co-authored-by: Yi Zhang <yizhang5@microsoft.com>
Co-authored-by: Thalia Wang <58485997+wenxuan0923@users.noreply.github.com>
Co-authored-by: Ji'an Liu <jiliu8@microsoft.com>
Co-authored-by: Tong Chen <45081443+tonche@users.noreply.github.com>
Co-authored-by: Ace Eldeib <aleldeib@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jeremymeng jeremymeng jeremymeng approved these changes

@HarshaNalluru HarshaNalluru HarshaNalluru approved these changes

@ramya-rao-a ramya-rao-a Awaiting requested review from ramya-rao-a

@bterlson bterlson Awaiting requested review from bterlson bterlson is a code owner

@witemple-msft witemple-msft Awaiting requested review from witemple-msft

@xirzec xirzec Awaiting requested review from xirzec

@conniey conniey Awaiting requested review from conniey

@yunhaoling yunhaoling Awaiting requested review from yunhaoling

@richardpark-msft richardpark-msft Awaiting requested review from richardpark-msft

@jsquire jsquire Awaiting requested review from jsquire

Assignees
No one assigned
Labels
Event Hubs
Projects
None yet
Milestone
No milestone
6 participants
@deyaaeldeen @azure-sdk @jsquire @jeremymeng @HarshaNalluru @chradek