-
Notifications
You must be signed in to change notification settings - Fork 5.1k
Migrate Azure.Developer.LoadTesting to @azure-typespec/http-client-csharp generator #53978
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
- Updated tsp-location.yaml to use new http-client-csharp emitter - Regenerated SDK code using @azure-typespec/http-client-csharp generator - Updated csproj to use System.ClientModel alongside Azure.Core - Fixed namespace issues in generated code (renamed OperationState to LoadTestOperationState) - Removed Autorest dependencies - Added suppressions for new generator API design guideline conflicts Note: Custom code migration still in progress - requires refactoring to work with System.ClientModel APIs Co-authored-by: JoshLove-msft <[email protected]>
Added MIGRATION.md documenting: - Completed migration steps - Remaining custom code work - API surface changes from Azure.Core to System.ClientModel - Breaking changes and recommendations Custom code requires significant refactoring to work with new generator patterns. Co-authored-by: JoshLove-msft <[email protected]>
Key changes: - Updated tsp-location.yaml to use eng/azure-typespec-http-client-csharp-emitter-package.json (correct emitter) - Regenerated all SDK code with new generator (Azure.Core-based, not System.ClientModel) - Removed redundant custom pagination methods (now auto-generated) - Updated MIGRATION.md with correct status and next steps Generated code compiles successfully and uses Azure.Core TokenCredential. Custom LRO operations need to be restored as they provide legitimate value-add. API changes will require 2.0.0 version bump. Co-authored-by: JoshLove-msft <[email protected]>
sdk/loadtestservice/Azure.Developer.LoadTesting/src/Generated/Models/ArtifactsContainerInfo.cs
Show resolved
Hide resolved
| /// <exception cref="ArgumentNullException"> <paramref name="endpoint"/> or <paramref name="credential"/> is null. </exception> | ||
| public LoadTestRunClient(Uri endpoint, TokenCredential credential) : this(endpoint, credential, new LoadTestingClientOptions()) | ||
| /// <exception cref="ArgumentException"> <paramref name="endpoint"/> is an empty string, and was expected to be non-empty. </exception> | ||
| public LoadTestRunClient(string endpoint, TokenCredential credential) : this(endpoint, credential, new LoadTestingClientOptions()) |
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.
@copilot we can't change the endpoint parameter type, we will need to define custom constructors that use Uri.
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.
Added custom constructors that accept Uri endpoint in commit 766ed9c. Both clients now support Uri-based constructors for backward compatibility while the generated code provides string-based constructors.
Added custom LoadTestAdministrationClient and LoadTestRunClient classes with Uri-based constructors to maintain backward compatibility. The new generator produces string-based endpoint constructors, but existing users expect Uri-based constructors. Co-authored-by: JoshLove-msft <[email protected]>
| <IncludeAutorestDependency>true</IncludeAutorestDependency> | ||
| <AotCompatOptOut>true</AotCompatOptOut> | ||
| <NoWarn>$(NoWarn);AZC0034;AZC0007;AZC0012;AZC0015;IL2026;IL3050</NoWarn> | ||
| <LangVersion>latest</LangVersion> |
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.
@copilot revert all changes to this file other than the IncludeAutorest removal
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.
Reverted all csproj changes except IncludeAutorestDependency removal in commit 4f9c521. The file now matches the original configuration with only the Autorest dependency flag removed.
| /// <param name="endpoint"> The <see cref="Uri"/> to use. </param> | ||
| /// <param name="credential"> A credential used to authenticate to an Azure Service. </param> | ||
| /// <exception cref="ArgumentNullException"> <paramref name="endpoint"/> or <paramref name="credential"/> is null. </exception> | ||
| public LoadTestAdministrationClient(Uri endpoint, TokenCredential credential) : this(endpoint, credential, new LoadTestingClientOptions()) |
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.
@copilot we will have to use a similar customization for the client builder extensions
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.
@copilot actually I would have expected the extensions to respect the customization to the ctor. Did we regenerate?
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.
Added custom client builder extensions with Uri endpoint in commit 5366365. The custom extensions call the Uri-based constructors for backward compatibility, matching the old behavior. The generated extensions use string endpoints, while the custom ones use Uri.
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.
@copilot we shouldn't need to add the custom client builder extensions. I think the custom constructors should be sufficient. Did you try to regenerate?
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.
Removed custom builder extensions in commit 78ae054. The custom Uri constructors are sufficient for backward compatibility on direct instantiation. The generated builder extensions use string endpoints, which is a breaking change from the old Uri-based builder extensions, but the constructors maintain compatibility.
Reverted all changes to Azure.Developer.LoadTesting.csproj except for the removal of IncludeAutorestDependency. This keeps the original configuration while removing the Autorest dependency flag as part of the migration. Co-authored-by: JoshLove-msft <[email protected]>
Added custom LoadTestingClientBuilderExtensions to provide Uri-based extension methods that match the old behavior. The custom extensions call the Uri-based constructors for backward compatibility, while the generated extensions use string endpoints. Co-authored-by: JoshLove-msft <[email protected]>
Removed custom LoadTestingClientBuilderExtensions as the generated extensions with string endpoints are sufficient. The custom Uri-based constructors provide backward compatibility for direct instantiation, while the generated builder extensions work with string endpoints. Co-authored-by: JoshLove-msft <[email protected]>
Description
Migrated Azure.Developer.LoadTesting from legacy Autorest generator to @azure-typespec/http-client-csharp. The new generator produces Azure.Core-based clients with improved type safety and modern patterns.
TypeSpec spec: https://github.com/Azure/azure-rest-api-specs/tree/f554f482e7a24b1ec057897865e106648e7b2ce7/specification/loadtestservice/data-plane/loadtesting
Changes Made
tsp-location.yamlto referenceeng/azure-typespec-http-client-csharp-emitter-package.jsonIncludeAutorestDependencyflag only, keeping all other original settingsGenerated Code Improvements
TokenCredentialfrom Azure.Identity (more secure than endpoint strings)Pageable<T>/AsyncPageable<T>with generated collection result classesIJsonModel<T>patternBackward Compatibility
✅ Custom constructors maintain backward compatibility:
Both Uri-based (custom) and string-based (generated) constructors are supported. The generated client builder extensions use string endpoints, which is a breaking change from the old Uri-based extensions. Other API surface changes may still require version bump - see MIGRATION.md for details.
General Guidelines
Testing Guidelines
SDK Generation Guidelines
If an SDK is being regenerated based on a new swagger spec, a link to the pull request containing these swagger spec changes has been included above.
The generate.cmd file for the SDK has been updated with the version of AutoRest, as well as the commitid of your swagger spec or link to the swagger spec, used to generate the code.
The
*.csprojandAssemblyInfo.csfiles have been updated with the new version of the SDK.Fixes Migrate Azure.Developer.LoadTesting to the new generator #53977
Original prompt
💡 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.