-
Notifications
You must be signed in to change notification settings - Fork 5k
Implement IJsonModel of DataFactoryElement<> for AOT compat #50447
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
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 implements the IJsonModel interface for DataFactoryElement to improve AOT (ahead-of-time) compatibility and adjust serialization behavior accordingly. Key changes include:
- Enhancing Utf8JsonWriterExtensions with a generic WriteObjectValue overload and modifying its switch cases to handle IJsonModel.
- Updating test logic in DataFactoryElementTests to use the new IPersistableModel-based serialization approach and modifying TestModel to implement IJsonModel.
- Refactoring serialization logic in DataFactoryKeyVaultSecret, DataFactoryElementJsonConverter, and DataFactoryElement.Serialization while introducing a new DataFactoryContext and adding a PackageReference for System.ClientModel.
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
sdk/core/Azure.Core/src/Shared/Utf8JsonWriterExtensions.cs | Updated method signature to include generic overload and added handling for IJsonModel. |
sdk/core/Azure.Core.Expressions.DataFactory/tests/DataFactoryElementTests.cs | Adjusted serialization calls in tests and updated TestModel to implement IJsonModel. |
sdk/core/Azure.Core.Expressions.DataFactory/src/DataFactoryKeyVaultSecret.Serialization.cs | Modified serialization logic to use IJsonModel for secret properties. |
sdk/core/Azure.Core.Expressions.DataFactory/src/DataFactoryElementJsonConverter.cs | Removed original Serialize methods and replaced Write override with a NotImplementedException. |
sdk/core/Azure.Core.Expressions.DataFactory/src/DataFactoryElement.cs | Converted to a partial class with additional warning suppressions. |
sdk/core/Azure.Core.Expressions.DataFactory/src/DataFactoryElement.Serialization.cs | Implemented IJsonModel and IPersistableModel for DataFactoryElement. |
sdk/core/Azure.Core.Expressions.DataFactory/src/DataFactoryContext.cs | Added a new context type for AOT-compatible model reading/writing. |
sdk/core/Azure.Core.Expressions.DataFactory/src/Azure.Core.Expressions.DataFactory.csproj | Added PackageReference to System.ClientModel. |
sdk/core/Azure.Core.Expressions.DataFactory/src/DataFactoryElementJsonConverter.cs
Outdated
Show resolved
Hide resolved
sdk/core/Azure.Core.Expressions.DataFactory/tests/DataFactoryElementTests.cs
Outdated
Show resolved
Hide resolved
…lementTests.cs Co-authored-by: Copilot <[email protected]>
…mentJsonConverter.cs Co-authored-by: Copilot <[email protected]>
sdk/core/Azure.Core.Expressions.DataFactory/src/Azure.Core.Expressions.DataFactory.csproj
Outdated
Show resolved
Hide resolved
API Change CheckAPIView identified API level changes in this PR and created the following API reviews |
Resolves #50403