-
Notifications
You must be signed in to change notification settings - Fork 5.1k
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
[Hub Generated] Review request for Microsoft.ServiceFabric to add version preview/2023-07-01-preview #25002
Conversation
Hi, @a-santamaria! Thank you for your pull request. To help get your PR merged: Generated ApiView comment added to this PR. You can use ApiView to show API versions diff. |
Swagger Validation Report
|
compared swaggers (via Oad v0.10.4)] | new version | base version |
---|---|---|
managedapplication.json | 2023-07-01-preview(8f31050) | 2022-01-01(main) |
managedapplication.json | 2023-07-01-preview(8f31050) | 2023-03-01-preview(main) |
managedcluster.json | 2023-07-01-preview(8f31050) | 2022-01-01(main) |
managedcluster.json | 2023-07-01-preview(8f31050) | 2023-03-01-preview(main) |
nodetype.json | 2023-07-01-preview(8f31050) | 2022-01-01(main) |
nodetype.json | 2023-07-01-preview(8f31050) | 2023-03-01-preview(main) |
The following breaking changes are detected by comparison with the latest stable version:
The following breaking changes are detected by comparison with the latest preview version:
Rule | Message |
---|---|
The new version is missing a path that was found in the old version. Was path '/subscriptions/{subscriptionId}/resourcegroups/{resourceGroupName}/providers/Microsoft.ServiceFabric/managedClusters' removed or restructured? Old: Microsoft.ServiceFabric/preview/2023-03-01-preview/managedcluster.json#L37:5 |
|
The new version is missing a path that was found in the old version. Was path '/subscriptions/{subscriptionId}/resourcegroups/{resourceGroupName}/providers/Microsoft.ServiceFabric/managedClusters/{clusterName}/nodeTypes' removed or restructured? Old: Microsoft.ServiceFabric/preview/2023-03-01-preview/nodetype.json#L37:5 |
️️✔️
CredScan succeeded [Detail] [Expand]
There is no credential detected.
️⚠️
LintDiff: 3 Warnings warning [Detail]
compared tags (via openapi-validator v2.1.4) | new version | base version |
---|---|---|
package-2023-07-preview | package-2023-07-preview(8f31050) | default(main) |
[must fix]The following errors/warnings are introduced by current PR:
Rule | Message | Related RPC [For API reviewers] |
---|---|---|
OperationId should contain the verb: 'getmaintenancewindowstatus' in:'managedMaintenanceWindowStatus_Get'. Consider updating the operationId Location: Microsoft.ServiceFabric/preview/2023-07-01-preview/managedcluster.json#L396 |
||
OperationId should contain the verb: 'applymaintenancewindow' in:'managedApplyMaintenanceWindow_Post'. Consider updating the operationId Location: Microsoft.ServiceFabric/preview/2023-07-01-preview/managedcluster.json#L439 |
||
Do not have duplicate name of x-ms-example, make sure every x-ms-example name unique. Duplicate x-ms-example: Maintenance Window Status Location: Microsoft.ServiceFabric/preview/2023-07-01-preview/managedcluster.json#L454 |
The following errors/warnings exist before current PR submission:
Rule | Message |
---|---|
GetCollectionResponseSchema |
The response in the GET collection operation 'ManagedClusterVersion_List' does not match the response definition in the individual GET operation 'ManagedClusterVersion_Get' . Location: Microsoft.ServiceFabric/preview/2023-07-01-preview/managedcluster.json#L561 |
GetCollectionResponseSchema |
The response in the GET collection operation 'ManagedClusterVersion_ListByEnvironment' does not match the response definition in the individual GET operation 'ManagedClusterVersion_GetByEnvironment' . Location: Microsoft.ServiceFabric/preview/2023-07-01-preview/managedcluster.json#L601 |
OperationId should contain the verb: 'getazresiliencystatus' in:'managedAzResiliencyStatus_Get'. Consider updating the operationId Location: Microsoft.ServiceFabric/preview/2023-07-01-preview/managedcluster.json#L353 |
️❌
Avocado: 2 Errors, 0 Warnings failed [Detail]
Rule | Message |
---|---|
MISSING_APIS_IN_DEFAULT_TAG |
The default tag should contain all APIs. The API path /subscriptions/{}/resourcegroups/{}/providers/Microsoft.ServiceFabric/managedClusters is not in the default tag. Please make sure the missing API swaggers are in the default tag.readme: specification/servicefabricmanagedclusters/resource-manager/readme.md json: Microsoft.ServiceFabric/preview/2023-03-01-preview/managedcluster.json |
MISSING_APIS_IN_DEFAULT_TAG |
The default tag should contain all APIs. The API path /subscriptions/{}/resourcegroups/{}/providers/Microsoft.ServiceFabric/managedClusters/{}/nodeTypes is not in the default tag. Please make sure the missing API swaggers are in the default tag.readme: specification/servicefabricmanagedclusters/resource-manager/readme.md json: Microsoft.ServiceFabric/preview/2023-03-01-preview/nodetype.json |
️️✔️
SwaggerAPIView succeeded [Detail] [Expand]
️️✔️
TypeSpecAPIView succeeded [Detail] [Expand]
️️✔️
ModelValidation succeeded [Detail] [Expand]
Validation passes for ModelValidation.
️️✔️
SemanticValidation succeeded [Detail] [Expand]
Validation passes for SemanticValidation.
️️✔️
PoliCheck succeeded [Detail] [Expand]
Validation passed for PoliCheck.
️️✔️
PrettierCheck succeeded [Detail] [Expand]
Validation passes for PrettierCheck.
️️✔️
SpellCheck succeeded [Detail] [Expand]
Validation passes for SpellCheck.
️️✔️
Lint(RPaaS) succeeded [Detail] [Expand]
Validation passes for Lint(RPaaS).
️️✔️
PR Summary succeeded [Detail] [Expand]
Validation passes for Summary.
️️✔️
Automated merging requirements met succeeded [Detail] [Expand]
Swagger Generation Artifacts
|
Generated ApiView
|
Hi @a-santamaria! The automation detected breaking changes in this pull request. As a result, it added the |
/azp run unifiedPipeline |
No commit pushedDate could be found for PR 25002 in repo Azure/azure-rest-api-specs |
/azp run |
Azure Pipelines successfully started running 2 pipeline(s). |
/azp run unifiedPipeline |
No pipelines are associated with this pull request. |
/azp run unifiedPipeline |
No pipelines are associated with this pull request. |
/azp run |
No commit pushedDate could be found for PR 25002 in repo Azure/azure-rest-api-specs |
@TimLovellSmith the linter error in [must fix] is: @rkmanda indicated that this is: "False alarm, we fixed this in the staging pipeline and will upgrade to production soon" |
/pr RequestMerge |
@a-santamaria - Please make sure the R has met the auto merging requirements - https://github.com/Azure/azure-rest-api-specs/pull/25002/checks?check_run_id=15921553176 |
Not ready for merge. Please review the requirements https://github.com/Azure/azure-rest-api-specs/pull/25002/checks?check_run_id=15921553176 |
Automatic PR validation restarted. This comment will be populated with next steps to merge this PR once validation is completed. Please wait ⌛. |
@sjanamma below is the explanation why the checks are failing: ❌ Swagger LintDiff: ❌ Swagger Avocado:
this is because we rename the path from resourcegroups to resourceGroups to fix the casing. We already got approval for this ❌ Approved-Suppression on put request that are long running operations we were getting flagged because we don't return 201. The model we have is described below (which we validated with ARM when we initially created it): For new cluster, return 200 with ProvisioningState = Creating @rkmanda, suggested to add add this suppression "If your service is old and you are carrying forward the 202 pattern, add a suppression and we will approve it." |
/azp run |
Azure Pipelines successfully started running 2 pipeline(s). |
@a-santamaria the Avocado check is no longer required to merge a PR. You can confirm this by observing the check |
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.
approved for merging
Swagger pipeline restarted successfully, please wait for status update in this comment. |
…sion preview/2023-07-01-preview (#25002) * copy from 2023-03-01-preview * update specs with api 2023-07-01-preview * update examples * remove ddosProtectionPlanId * fix example names * run prettier * fix ManagedApplyMaintenanceWindowPut_example response * fix spell check * add headers in 202 responses * fix cSpell path * fix casting * add suppressions rules * add ddosProtectionPlanId * fix ARM comments --------- Co-authored-by: Alfredo Santamaria Gomez <alsantam@microsoft.com>
This is a PR generated at OpenAPI Hub. You can view your work branch via this link.
ARM (Control Plane) API Specification Update Pull Request
PR review workflow diagram
Please understand this diagram before proceeding. It explains how to get your PR approved & merged.
Purpose of this PR
What's the purpose of this PR? Check all that apply. This is mandatory!
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:
ARM resource provider contract and
REST guidelines (estimated time: 4 hours).
I understand this is required before I can request review from an ARM API Review board.
ARM API changes review
ARMReview
label.ARMReview
label, if appropriate.If this happens, proceed according to guidance given in GitHub comments also added by the automation.
Breaking change review
If you have any breaking changes as defined in the Breaking Change Policy,
follow the process outlined in the High-level Breaking Change Process doc.
Getting help
and https://aka.ms/ci-fix.