-
Notifications
You must be signed in to change notification settings - Fork 1k
Make sync client api obsolete #3174
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
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #3174 +/- ##
==========================================
+ Coverage 57.88% 58.01% +0.13%
==========================================
Files 353 354 +1
Lines 80549 77908 -2641
Branches 14037 13816 -221
==========================================
- Hits 46628 45201 -1427
+ Misses 29614 28471 -1143
+ Partials 4307 4236 -71 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
Pull Request Overview
This PR makes the synchronous client API obsolete by adding [Obsolete] attributes to sync methods and converting all test code to use the async equivalents. The changes ensure all client operations use async patterns while maintaining backward compatibility through obsolete sync methods that internally call the async versions.
- Marks sync client methods as obsolete and implements them as wrappers around async methods
- Converts all test code from sync to async implementations
- Removes conditional compilation directives (
CLIENT_ASYNC,NET_STANDARD_ASYNC) as async is now the default - Updates project files to use the
NET_STANDARD_OBSOLETE_SYNCdefine
Reviewed Changes
Copilot reviewed 51 out of 59 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| Tests/Opc.Ua.Client.Tests/SessionClientBatchTest.cs | Removes sync test methods, keeps only async versions |
| Tests/Opc.Ua.Client.Tests/ReverseConnectTest.cs | Converts sync method calls to async equivalents |
| Tests/Opc.Ua.Client.Tests/RequestHeaderTest.cs | Updates test method to use async ReadValuesAsync |
| Tests/Opc.Ua.Client.Tests/Opc.Ua.Client.Tests.csproj | Removes CLIENT_ASYNC define constant |
| Tests/Opc.Ua.Client.Tests/NodeCacheAsyncTest.cs | Simplifies by removing sync/async branching logic |
| Tests/Opc.Ua.Client.Tests/DurableSubscriptionTest.cs | Converts subscription operations to async patterns |
| Tests/Opc.Ua.Client.Tests/ContinuationPointInBatchTest.cs | Updates browse operations to async methods |
| Tests/Opc.Ua.Client.Tests/ClientTestServices.cs | Wraps sync interface methods with async calls |
| Tests/Opc.Ua.Client.Tests/ClientTestServerQuotas.cs | Removes sync version of chunk reading test |
| Tests/Opc.Ua.Client.Tests/ClientTestNoSecurity.cs | Converts discovery and session operations to async |
| Tests/Opc.Ua.Client.Tests/ClientTestFramework.cs | Updates operation limit reading to async |
| Tests/Opc.Ua.Client.Tests/ClientTest.cs | Extensive conversion of sync session operations to async |
| Tests/Opc.Ua.Client.Tests/ClientFixture.cs | Updates endpoint selection to async |
| Tests/Opc.Ua.Client.ComplexTypes.Tests/TypeSystemClientTest.cs | Converts node cache operations to async |
| Tests/Opc.Ua.Client.ComplexTypes.Tests/NodeCacheResolverTests.cs | Updates browser operations to async |
| Tests/Opc.Ua.Client.ComplexTypes.Tests/DataDictionaryTests.cs | Converts dictionary loading to async |
| Stack/Opc.Ua.Core/Stack/Configuration/ConfiguredEndpoints.cs | Marks sync UpdateFromServer methods as obsolete |
| Stack/Opc.Ua.Core/Stack/Client/SessionClient.cs | Minor code formatting change |
| Stack/Opc.Ua.Core/Stack/Client/IClientBase.cs | Removes sync Close method from interface |
| Stack/Opc.Ua.Core/Stack/Client/DiscoveryClient.cs | Marks sync discovery methods as obsolete |
| Stack/Opc.Ua.Core/Stack/Client/ClientBase.cs | Removes sync Close implementation |
| Stack/Opc.Ua.Core/Security/Certificates/CertificateTrustList.cs | Adds cancellation token to async method |
| Stack/Opc.Ua.Core/Opc.Ua.Core.csproj | Adds NET_STANDARD_OBSOLETE_SYNC define |
| Libraries/Opc.Ua.Server/Server/StandardServer.cs | Converts discovery registration to async |
| Libraries/Opc.Ua.Server/NodeManager/MasterNodeManager.cs | Updates method call handling to async |
| Libraries/Opc.Ua.Gds.Client.Common/ServerPushConfigurationClient.cs | Updates endpoint selection to async |
| Libraries/Opc.Ua.Gds.Client.Common/GlobalDiscoveryServerClient.cs | Updates endpoint selection to async |
| Libraries/Opc.Ua.Client/Subscription/SubscriptionObsolete.cs | New file containing obsolete wrappers for sync methods |
| Libraries/Opc.Ua.Client/Subscription/SubscriptionAsync.cs | Removed file (async methods moved to main class) |
| Libraries/Opc.Ua.Client/Subscription/Subscription.cs | Merges async methods into main class, removes sync implementations |
| Libraries/Opc.Ua.Client/Subscription/MonitoredItem.cs | Converts GetEventType to async |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
romanett
left a comment
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.
Great addtion to streamline our codebase. Also like that the deprecation of sync & APM based generated code is already prepared / done
Proposed changes
Describe the changes here to communicate to the maintainers why we should accept this pull request. If it fixes a bug or resolves a feature request, be sure to link to that issue.
Related Issues
Types of changes
What types of changes does your code introduce?
Put an
xin the boxes that apply. You can also fill these out after creating the PR.Checklist
Put an
xin the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.Further comments
If this is a relatively large or complex change, kick off the discussion by explaining why you chose the solution you did and what alternatives you considered, etc...