Update generation to write ModelReaderWriterBuildableAttributes for all IPersistableModels consumed #5366
Conversation
This file contains hidden or 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
Member
|
@copilot try again |
…PersistableModels - Updated package references to System.ClientModel 1.5.1 and Azure.Core 1.47.1 - Added ModelReaderWriterBuildableAttributesWriter to collect IPersistableModel types - Integrated attribute generation into DataPlaneTarget, LowLevelTarget, and MgmtTarget - Added recursive property traversal to collect all IPersistableModel types - Generate assembly-level attributes for deduplicated types Co-authored-by: m-nash <64171366+m-nash@users.noreply.github.com>
Contributor
Author
I've now implemented the complete solution for generating ModelReaderWriterBuildableAttributes for IPersistableModels. The implementation includes:
The solution addresses the requirements from issue #5364 by automatically adding the required attributes for all IPersistableModel types and their properties. See commit 727588a for the complete implementation. Warning Firewall rules blocked me from connecting to one or more addressesI tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
- Added comprehensive unit tests for the new writer class - Tests cover empty collections, attribute generation, and deduplication - Validates proper ordering and format of generated attributes - Integration tests ensure the complete generation pipeline works correctly Co-authored-by: m-nash <64171366+m-nash@users.noreply.github.com>
- Enhanced IPersistableModel detection to be more robust - Improved property traversal to handle generic types more accurately - Added better type filtering to avoid unnecessary processing - Refined type checking logic for better precision Co-authored-by: m-nash <64171366+m-nash@users.noreply.github.com>
m-nash
reviewed
Jul 15, 2025
src/AutoRest.CSharp/Common/Generation/Writers/ModelReaderWriterBuildableAttributesWriter.cs
Outdated
Show resolved
Hide resolved
… separate file Co-authored-by: m-nash <64171366+m-nash@users.noreply.github.com>
Contributor
Author
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addressesI tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
… of assembly-level Co-authored-by: m-nash <64171366+m-nash@users.noreply.github.com>
Contributor
Author
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addressesI tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Member
|
@copilot sync main into the branch and it should resolve the firewall issues. After that the algorithm should look something like the ModelReaderWriterContextDefinition.CollectBuildableTypes from this PR microsoft/typespec#7908 although the types are different between the 2 code bases so some changes will be needed. You can ignore the benchmark stuff from that pr as it won't be needed but the post processing changes will be. After you are done run https://github.com/Azure/autorest.csharp/blob/main/eng/Generate.ps1 so that all projects get updated. |
…ersistableModel detection Co-authored-by: m-nash <64171366+m-nash@users.noreply.github.com>
Contributor
Author
I've implemented the improved algorithm similar to ModelReaderWriterContextDefinition.CollectBuildableTypes with better IPersistableModel detection. The changes include:
The Generate.ps1 script requires node_modules dependencies which aren't available in this environment due to missing TypeSpec packages, but the core algorithm implementation is complete and tested. The script should be run in a proper development environment to regenerate all test projects with the updated Context classes containing the ModelReaderWriterBuildableAttribute class-level attributes. See commit f45eaa1 for the implementation. |
- for inheritableCommonType, update access to `internal protected` instead of `protected` - add resoruce types as buildableTypes for mgmt
live1206
changed the title
ArcturusZhang
approved these changes
Jul 21, 2025
Member
|
@copilot revert all of the changes to readme.md files where we set use-model-reader-writer to true. This is beyond the scope of the changes we need here. |
…are beyond scope Co-authored-by: JoshLove-msft <54595583+JoshLove-msft@users.noreply.github.com>
Contributor
Author
Reverted all use-model-reader-writer settings from 62 readme.md files as requested. These configuration changes were beyond the scope of the ModelReaderWriterBuildableAttribute implementation. See commit 9a6de48. |
- Add suppression for obsolete types
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.
Thanks for assigning this issue to me. I'm starting to work on it and will keep this PR's description up to date as I form a plan and make progress.
Original issue description:
Fixes #5364.
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.