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

Synchronize OperationInternal calls #28375

Merged
merged 18 commits into from
Apr 29, 2022
Merged

Synchronize OperationInternal calls #28375

merged 18 commits into from
Apr 29, 2022

Conversation

AlexanderSher
Copy link
Contributor

Changes OperationInternal / OperationInternal<T> so that only one call of UpdateStatusAsync and WaitForCompletionAsync can be executed concurrently. After OperationInternal / OperationInternal<T> is set in its final state, no more call to the service is made.

Implementation is updated based on the discussion in #27639

@AlexanderSher AlexanderSher mentioned this pull request Apr 25, 2022
Closed
@azure-sdk
Copy link
Collaborator

API change check for Azure.Security.ConfidentialLedger

API changes are not detected in this pull request for Azure.Security.ConfidentialLedger

@azure-sdk
Copy link
Collaborator

API change check for Microsoft.Azure.Data.SchemaRegistry.ApacheAvro

API changes are not detected in this pull request for Microsoft.Azure.Data.SchemaRegistry.ApacheAvro

@azure-sdk
Copy link
Collaborator

API change check for Azure.AI.TextAnalytics

API changes are not detected in this pull request for Azure.AI.TextAnalytics

@azure-sdk
Copy link
Collaborator

API change check for Azure.Security.KeyVault.Certificates

API changes are not detected in this pull request for Azure.Security.KeyVault.Certificates

@azure-sdk
Copy link
Collaborator

API change check for Azure.Security.KeyVault.Keys

API changes are not detected in this pull request for Azure.Security.KeyVault.Keys

@azure-sdk
Copy link
Collaborator

API change check for Azure.Security.KeyVault.Secrets

API changes are not detected in this pull request for Azure.Security.KeyVault.Secrets

@azure-sdk
Copy link
Collaborator

API change check for Azure.Core

API changes are not detected in this pull request for Azure.Core

@azure-sdk
Copy link
Collaborator

API change check for Azure.ResourceManager.Network

API changes are not detected in this pull request for Azure.ResourceManager.Network

@AlexanderSher AlexanderSher mentioned this pull request Apr 27, 2022
Closed
11 tasks
kinelski
kinelski approved these changes Apr 29, 2022
View reviewed changes
Copy link
Member

@kinelski kinelski left a comment

Choose a reason for hiding this comment

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

LGTM

@AlexanderSher AlexanderSher dismissed heaths’s stale review April 29, 2022 21:37

Talked to @heaths offline, he has no more issues with this PR

@AlexanderSher AlexanderSher merged commit 5b1d4f9 into Azure:main Apr 29, 2022
@AlexanderSher AlexanderSher deleted the SyncOperationInternal branch May 1, 2022 01:55
@AlexanderSher AlexanderSher mentioned this pull request May 2, 2022
Closed
paterasMSFT pushed a commit that referenced this pull request Jun 15, 2022
@AlexanderSher @paterasMSFT
Synchronize OperationInternal calls (#28375)
9153155 
* Initial commit

* Minimize code duplication

* Fix tests

* Fix OperationInternal

* Use AsyncLockWithValue from Core

* Fix operation naming

* Address PR comments

* Address PR comment

* More tests

* Fix one test by hand

* More fixed test records

* More fixed test records

* More fixed test records

* More fixed test records

* Fix test records

* Add tests

* Added comments

* Address CR comments
zhihaoxue pushed a commit to zhihaoxue/azure-sdk-for-net that referenced this pull request Jul 27, 2022
@AlexanderSher @zhihaoxue
Synchronize OperationInternal calls (Azure#28375)
a60de8b 
* Initial commit

* Minimize code duplication

* Fix tests

* Fix OperationInternal

* Use AsyncLockWithValue from Core

* Fix operation naming

* Address PR comments

* Address PR comment

* More tests

* Fix one test by hand

* More fixed test records

* More fixed test records

* More fixed test records

* More fixed test records

* Fix test records

* Add tests

* Added comments

* Address CR comments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@christothes christothes christothes left review comments

@KrzysztofCwalina KrzysztofCwalina KrzysztofCwalina left review comments

@JoshLove-msft JoshLove-msft JoshLove-msft left review comments

@kinelski kinelski kinelski approved these changes

@annelo-msft annelo-msft annelo-msft approved these changes

@chidozieononiwu chidozieononiwu Awaiting requested review from chidozieononiwu

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

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

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

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

@maririos maririos Awaiting requested review from maririos

@m-nash m-nash Awaiting requested review from m-nash

@heaths heaths Awaiting requested review from heaths

@fengzhou-msft fengzhou-msft Awaiting requested review from fengzhou-msft

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

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

@Yao725 Yao725 Awaiting requested review from Yao725

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

Assignees
No one assigned
Labels
Azure.Core Cognitive - Text Analytics KeyVault Schema Registry
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@AlexanderSher @azure-sdk @christothes @heaths @kinelski @KrzysztofCwalina @annelo-msft @JoshLove-msft