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.

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

[Azure Load Testing] Onboarding to TypeSpec for control plane GA APIs #27565

Merged

Conversation

Himanshu49
Copy link
Member

ARM (Control Plane) API Specification Update Pull Request

Tip

Overwhelmed by all this guidance? See the Getting help section at the bottom of this PR description.

Note

As of January 2024 there is no PR assignee. This is expected. See https://aka.ms/azsdk/pr-arm-review.

PR review workflow diagram

Please understand this diagram before proceeding. It explains how to get your PR approved & merged.

diagram

Click here to see the details of Step 1

Breaking changes review (Diagram Step 1)

If the automation determines you have breaking changes, i.e. Step 1 from the diagram applies to you,
you must follow the breaking changes process.
IMPORTANT This applies even if:

  • The tool fails while it shouldn't, e.g. due to runtime exception, or incorrect detection of breaking changes.
  • You believe there is no need for you to request breaking change approval, for any reason.
    Such claims must be reviewed, and the process is the same.
Click here to see the details of Step 2

ARM API changes review (Diagram Step 2)

Click here to see the diagram footnotes

Diagram footnotes

[1] See ARM review queue (for PR merge queues, see [2]).
[2] public repo merge queue, private repo merge queue (for ARM review queue, [1])
The ARM reviewer on-call engineer visits the merge queue twice a day, so the approximate ETA for merges is 12 - 24 hours.

Purpose of this PR

What's the purpose of this PR? Check the specific option that applies. This is mandatory!

  • New resource provider.
  • New API version for an existing resource provider. (If API spec is not defined in TypeSpec, the PR should have been generated using OpenAPI Hub).
  • Update existing version for a new feature. (This is applicable only when you are revising a private preview API version.)
  • Update existing version to fix OpenAPI spec quality issues in S360.
  • Other, please clarify:
    • _We are onboarding our existing control plane GA Spec to be generated with the help of TypeSpec. No changes to APIs in our service were made. _

Due diligence checklist

To merge this PR, you must go through the following checklist and confirm you understood
and followed the instructions by checking all the boxes:

  • I confirm this PR is modifying Azure Resource Manager (ARM) related specifications, and not data plane related specifications.
  • I have reviewed following Resource Provider guidelines, including
    ARM resource provider contract and
    REST guidelines (estimated time: 4 hours).
    I understand this is required before I can proceed to the diagram Step 2, "ARM API changes review", for this PR.

Additional information

Viewing API changes

For convenient view of the API changes made by this PR, refer to the URLs provided in the table
in the Generated ApiView comment added to this PR. You can use ApiView to show API versions diff.

Suppressing failures

If one or multiple validation error/warning suppression(s) is detected in your PR, please follow the
suppressions guide to get approval.

Getting help

  • First, please carefully read through this PR description, from top to bottom. Please fill out the Purpose of this PR and Due diligence checklist.
  • To understand what you must do next to merge this PR, see the Next Steps to Merge comment. It will appear within few minutes of submitting this PR and will continue to be up-to-date with current PR state.
  • For guidance on fixing this PR CI check failures, see the hyperlinks provided in given failure
    and https://aka.ms/ci-fix.
  • For help with PR workflow diagram Step 2 (ARM review), see https://aka.ms/azsdk/pr-arm-review.
  • If the PR CI checks appear to be stuck in queued state, please add a comment with contents /azp run.
    This should result in a new comment denoting a PR validation pipeline has started and the checks should be updated after few minutes.
  • If the help provided by the previous points is not enough, post to https://aka.ms/azsdk/support/specreview-channel and link to this PR.

catalinaperalta and others added 30 commits June 1, 2023 17:32
* Update the diff for put and patch operation

* Change LoadTestAdministrationOperations to LoadTestAdministration to align with the swagger LoadTestAdministration_ prefix

Change LoadTestRunOperations to LoadTestRun to align with the swagger LoadTestRun_ prefix
* Update the projected name

* Renamed

* update config file

* update folder

* Remove client.tsp

* Update the projected name to old ones

* Adjust the positions

* Update client.tsp

* Update tspconfig.yaml

Added correct options for python emitter.

* Update the metadata

* Change the projectedName to client.tsp

* Remove the projectedName in routes.tsp

* update file

* Update the .Net config

* Remove the projectedName change for load testing

* Revert position change

* Update specification/loadtestservice/LoadTestService/tspconfig.yaml

---------

Co-authored-by: Ray Chen <[email protected]>
* python config

* Update client.tsp
* java config

* project for java

* metricNamespace is required

* aggregation is optional

* body is optional

* metricName and timespan is required
@catalinaperalta
Copy link
Member

@Himanshu49 Typespec validation is failing, can you run the tsv command locally and fix any errors that show up?

…Properties model in Azure.ResourceManager.Foundations
@krisnaray krisnaray merged commit 3e81da3 into Azure:main Apr 3, 2024
31 of 32 checks passed
wiboris pushed a commit to wiboris/azure-rest-api-specs that referenced this pull request May 7, 2024
…Azure#27565)

* initial tsp

* client.tsp

* client.tsp improvements

* model updates

* update routes

* update implementation

* Sync typespec and swagger change for loadtesting service (Azure#24412)

* Update the diff for put and patch operation

* Change LoadTestAdministrationOperations to LoadTestAdministration to align with the swagger LoadTestAdministration_ prefix

Change LoadTestRunOperations to LoadTestRun to align with the swagger LoadTestRun_ prefix

* update models

* add list metric dimensions op and fix create test run

* update client.tsp

* fix DimensionValueList and update TestRun model

* Configuration changes for LoadTesting (Azure#24481)

* Update the projected name

* Renamed

* update config file

* update folder

* Remove client.tsp

* Update the projected name to old ones

* Adjust the positions

* Update client.tsp

* Update tspconfig.yaml

Added correct options for python emitter.

* Update the metadata

* Change the projectedName to client.tsp

* Remove the projectedName in routes.tsp

* update file

* Update the .Net config

* Remove the projectedName change for load testing

* Revert position change

* Update specification/loadtestservice/LoadTestService/tspconfig.yaml

---------

Co-authored-by: Ray Chen <[email protected]>

* update routes to be more accurate

* update client.tsp for python

* python config to avoid breaking (Azure#24801)

* python config

* Update client.tsp

* java, feature/loadtesting, match tsp and swagger (Azure#24878)

* java config

* project for java

* metricNamespace is required

* aggregation is optional

* body is optional

* metricName and timespan is required

* Changes for 2023-04-01-preview version

* Minor refactoring

* Added URL based changes and versioning support (versioning not working)

* Versioning support changes

* Refactoring

* Refactoring

* config changes

* Added examples and few refactoring

* Adding emitters for pythin, ts and csharp

* Updated description for timespan field

* Update tspconfig as validation pipelines failing.

* Rename createOrUpdateTest.json to CreateOrUpdateTest.json

* Rename createOrUpdateTestRun.json to CreateOrUpdateTestRun.json

* Rename createOrUpdateTest.json to CreateOrUpdateTest.json

* Rename createOrUpdateTestRun.json to CreateOrUpdateTestRun.json

* suppress warning in client.tsp

* Updated examples for stable version

* Updated examples based on latest changes

* Updated spec for 2023-04-01-preview

* Added description for APIVersions model

* Updated readme and added examples

* removed tsp-output folder and files in it

* Updated service name in client

* Updated api version parameter

* Pushing updated json

* Removing unused example file

* suppressing warnings in client.tsp

* Changes to use standard templates for operations

* Changing create and update to generic template as it is changing to default resource parameter.

* Updating visibility of FileName

* Minor change in name

* changes in visibility of filename

* Change in model name of fileInfo

* Changing file info to testfileinfo

* Updated jsons

* Changes to rectify warning in pipeline

* changing metricName to metricname

* Updated examples to revert to metricname

* Ran tsp format and update example for metricname parameter

* Added query parameter for list test and testrun operation

* Updated description

* Enable linter rules

* Updated description of models and operations

* Updated specs after internal review of APIs

* Added ContainerInfo in 2023-04-01-preview

* Updated operation id for stop test run

* Updated operation id for stop test run

* Updated default values of properties

* Description changes for few properties and models

* Addressed errors in breaking change pipeline

* Removed unused models, refactored code, addressed PR comments

* nit: changes in client and model tsp

* spec after nit changes

* Changes suggested in API review

* Update specification/loadtestservice/LoadTestService/client.tsp

Co-authored-by: Timothee Guerin <[email protected]>

* Initial draft for control plane spec using TypeSpec

* Suggested changes in API review

* updating name and description for ContainerInfo

* Added publicIPDisabled property to Test and TestRun model in 2023-04-01-preview version

* Update properties in examples JSON files

* Linting changes

* Adding outbound and checkAvailability APIs

* Added outboundNetworkDependenciesEndpoint and checkAvailability operations

* Removed example tag and related imports

* Minor linting

* Merge main and changes after run npx command in local

* Refactoring code

* Addressing PR comments

* Changes based on diff with existing swagger

* nit

* nit changes after merge

* Added extension x-ms-parameter-location to loadTestName parameter

* Resolved outboundNetworkDependenciesEndpoints API

* Added examples to directory

* minor changes based on pipeline validation failure

* Fixing CI failures in PR

* fixed LoadTestResourcePatchRequestBody

* Changes to remove unwanted headers

* Fixing required properties

* Fixing CI failures

* Fixing CI errors

* Removed retry-after header from responses

* Removing breaking changes

* Fixing CI errors on PR

* Resolving SDK breaking changes

* Making value as optional in list response

* linting by npx tsv

* Changes based on PR comments

* Removing breaking changes

* Removed duplicate Resource model issue

* Addressing PR comments

* Updated model and routes to use templates and go for breaking changes in "value" making it required.

* Updating location parameter to be used as common-types

* removed 200 response for delete

* Addressed PR comments, removed duplication of standard models

* nit: addressing PR comments

* removed @azure-tools/typespec-providerhub

* update after npx tsv

* Create sdk-suppressions.yaml

* add azure-sdk-for-go suppression

* Update sdk-suppressions.yaml

* nit: json updated

* nit: tsp compile

* Addressed PR comments. Changed common type version from v5 to v3

* Using version v5 for common types due to warning from ManagedIdentityProperties model in Azure.ResourceManager.Foundations

---------

Co-authored-by: Catalina Peralta <[email protected]>
Co-authored-by: Mary Gao <[email protected]>
Co-authored-by: Ray Chen <[email protected]>
Co-authored-by: Yuchao Yan <[email protected]>
Co-authored-by: Weidong Xu <[email protected]>
Co-authored-by: Mike Harder <[email protected]>
Co-authored-by: Himanshu Bisht <[email protected]>
Co-authored-by: Timothee Guerin <[email protected]>
Co-authored-by: Alancere <[email protected]>
Co-authored-by: kazrael2119 <[email protected]>
Co-authored-by: Mark Cowlishaw <[email protected]>

#suppress "@azure-tools/typespec-azure-core/no-nullable" "We need resourceId to be nullable."
@doc("User assigned identity to use for accessing key encryption key Url. Ex: /subscriptions/fa5fc227-a624-475e-b696-cdd604c735bc/resourceGroups/<resource group>/providers/Microsoft.ManagedIdentity/userAssignedIdentities/myId.")
resourceId?: string | null;
Copy link
Member

Choose a reason for hiding this comment

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

Can I confirm this value would be allowed with null in the back end?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ARMReview ARMSignedOff <valid label in PR review process>add this label when ARM approve updates after review BreakingChange-Approved-Benign Changes are not breaking at the REST API level and have at most minor impact to generated SDKs. BreakingChange-Go-Sdk-Suppression BreakingChange-Go-Sdk-Suppression-Approved BreakingChange-JavaScript-Sdk-Suppression BreakingChange-JavaScript-Sdk-Suppression-Approved BreakingChange-Python-Sdk-Suppression BreakingChange-Python-Sdk-Suppression-Approved BreakingChangeReviewRequired <valid label in PR review process>add this label when breaking change review is required IDCDevDiv ReadyForApiTest <valid label in PR review process>add this label when swagger and service APIs are ready for test resource-manager RPaaS TypeSpec Authored with TypeSpec
Projects
None yet
Development

Successfully merging this pull request may close these issues.