Update appconfiguration.json to include the "operations" path by HarshaNalluru · Pull Request #23125 · Azure/azure-rest-api-specs

HarshaNalluru · 2023-03-16T04:00:09Z

Data Plane API - Pull Request

Followup of #22840
Updates the swagger to include the "operations" path that is necessary for the lro create polling operation.

openapi-workflow-bot · 2023-03-16T04:00:13Z

Hi, @HarshaNalluru Thanks for your PR. I am workflow bot for review process. Here are some small tips.

Please ensure to do self-check against checklists in first PR comment.

PR assignee is the person auto-assigned and responsible for your current PR reviewing and merging.

For specs comparison cross API versions, Use API Specs Comparison Report Generator

If there is CI failure(s), to fix CI error(s) is mandatory for PR merging; or you need to provide justification in PR comment for explanation. How to fix?

Any feedback about review process or workflow bot, pls contact swagger and tools team. vscswagger@microsoft.com

openapi-pipeline-app · 2023-03-16T04:00:13Z

Swagger Validation Report

️❌BreakingChange: 1 Errors, 0 Warnings failed [Detail]

compared swaggers (via Oad v0.10.4)]	new version	base version
appconfiguration.json	2022-11-01-preview(48125b4)	2022-11-01-preview(main)

Rule	Message
`❌` 1038 - AddedPath	The new version is adding a path that was not found in the old version. New: Microsoft.AppConfiguration/preview/2022-11-01-preview/appconfiguration.json#L1486:5

️️✔️Breaking Change(Cross-Version) succeeded [Detail] [Expand]

There are no breaking changes.

️️✔️CredScan succeeded [Detail] [Expand]

There is no credential detected.

️⚠️LintDiff: 4 Warnings warning [Detail]

compared tags (via openapi-validator v2.0.0)	new version	base version
package-2022-11-01-preview	package-2022-11-01-preview(48125b4)	package-2022-11-01-preview(main)

[must fix]The following errors/warnings are introduced by current PR:

Rule	Message	Related RPC [For API reviewers]
⚠️ OperationId	OperationId should be of the form 'Noun_Verb' Location: Microsoft.AppConfiguration/preview/2022-11-01-preview/appconfiguration.json#L1492
⚠️ ErrorResponse	Error response should contain a x-ms-error-code header. Location: Microsoft.AppConfiguration/preview/2022-11-01-preview/appconfiguration.json#L1516
⚠️ ErrorResponse	Error response schema should contain an object property named `error`. Location: Microsoft.AppConfiguration/preview/2022-11-01-preview/appconfiguration.json#L1518
⚠️ SchemaTypeAndFormat	Schema with type: integer should specify format Location: Microsoft.AppConfiguration/preview/2022-11-01-preview/appconfiguration.json#L1518

The following errors/warnings exist before current PR submission:

Only 30 items are listed, please refer to log for more details.

Rule	Message
`❌` IntegerTypeMustHaveFormat	The integer type does not have a format, please add it. Location: Microsoft.AppConfiguration/preview/2022-11-01-preview/appconfiguration.json#L1763
`❌` XmsParameterLocation	The parameter 'ApiVersion' is defined in global parameters section without 'x-ms-parameter-location' extension. This would add the parameter as the client property. Please ensure that this is exactly you want. If so, apply the extension 'x-ms-parameter-location': 'client'. Else, apply the extension 'x-ms-parameter-location': 'method'. Location: Microsoft.AppConfiguration/preview/2022-11-01-preview/appconfiguration.json#L1905
`❌` HostParametersValidation	The host parameter must be typed 'type 'string', format 'url''. Location: Microsoft.AppConfiguration/preview/2022-11-01-preview/appconfiguration.json#L2018
⚠️ ListInOperationName	Since operation response has model definition in array type, it should be of the form '_list'. Location: Microsoft.AppConfiguration/preview/2022-11-01-preview/appconfiguration.json#L12
⚠️ OperationId	OperationId should be of the form 'Noun_Verb' Location: Microsoft.AppConfiguration/preview/2022-11-01-preview/appconfiguration.json#L17
⚠️ OperationId	OperationId for get method on a collection should contain 'List' Location: Microsoft.AppConfiguration/preview/2022-11-01-preview/appconfiguration.json#L17
⚠️ ParameterNamesConvention	Parameter name 'After' should be camel case. Location: Microsoft.AppConfiguration/preview/2022-11-01-preview/appconfiguration.json#L37
⚠️ PaginationResponse	Response body schema of pageable response should contain top-level array property `value` Location: Microsoft.AppConfiguration/preview/2022-11-01-preview/appconfiguration.json#L52
⚠️ ErrorResponse	Error response should contain a x-ms-error-code header. Location: Microsoft.AppConfiguration/preview/2022-11-01-preview/appconfiguration.json#L62
⚠️ ErrorResponse	Error response schema should contain an object property named `error`. Location: Microsoft.AppConfiguration/preview/2022-11-01-preview/appconfiguration.json#L64
⚠️ SchemaTypeAndFormat	Schema with type: integer should specify format Location: Microsoft.AppConfiguration/preview/2022-11-01-preview/appconfiguration.json#L64
⚠️ OperationId	OperationId should be of the form 'Noun_Verb' Location: Microsoft.AppConfiguration/preview/2022-11-01-preview/appconfiguration.json#L84
⚠️ ParameterNamesConvention	Parameter name 'After' should be camel case. Location: Microsoft.AppConfiguration/preview/2022-11-01-preview/appconfiguration.json#L101
⚠️ SuccessResponseBody	All success responses except 202 & 204 should define a response body. Location: Microsoft.AppConfiguration/preview/2022-11-01-preview/appconfiguration.json#L114
⚠️ ErrorResponse	Error response should have a schema. Location: Microsoft.AppConfiguration/preview/2022-11-01-preview/appconfiguration.json#L123
⚠️ ErrorResponse	Error response should contain a x-ms-error-code header. Location: Microsoft.AppConfiguration/preview/2022-11-01-preview/appconfiguration.json#L123
⚠️ ListInOperationName	Since operation response has model definition in array type, it should be of the form '_list'. Location: Microsoft.AppConfiguration/preview/2022-11-01-preview/appconfiguration.json#L135
⚠️ OperationId	OperationId should be of the form 'Noun_Verb' Location: Microsoft.AppConfiguration/preview/2022-11-01-preview/appconfiguration.json#L140
⚠️ OperationId	OperationId for get method on a collection should contain 'List' Location: Microsoft.AppConfiguration/preview/2022-11-01-preview/appconfiguration.json#L140
⚠️ ParameterNamesConvention	Parameter name 'After' should be camel case. Location: Microsoft.AppConfiguration/preview/2022-11-01-preview/appconfiguration.json#L166
⚠️ ParameterNamesConvention	Parameter name '$Select' should not begin with '$' or '@'. Location: Microsoft.AppConfiguration/preview/2022-11-01-preview/appconfiguration.json#L177
⚠️ PaginationResponse	Response body schema of pageable response should contain top-level array property `value` Location: Microsoft.AppConfiguration/preview/2022-11-01-preview/appconfiguration.json#L190
⚠️ ErrorResponse	Error response should contain a x-ms-error-code header. Location: Microsoft.AppConfiguration/preview/2022-11-01-preview/appconfiguration.json#L200
⚠️ ErrorResponse	Error response schema should contain an object property named `error`. Location: Microsoft.AppConfiguration/preview/2022-11-01-preview/appconfiguration.json#L202
⚠️ SchemaTypeAndFormat	Schema with type: integer should specify format Location: Microsoft.AppConfiguration/preview/2022-11-01-preview/appconfiguration.json#L202
⚠️ OperationId	OperationId should be of the form 'Noun_Verb' Location: Microsoft.AppConfiguration/preview/2022-11-01-preview/appconfiguration.json#L222
⚠️ ParameterNamesConvention	Parameter name 'After' should be camel case. Location: Microsoft.AppConfiguration/preview/2022-11-01-preview/appconfiguration.json#L245
⚠️ ParameterNamesConvention	Parameter name '$Select' should not begin with '$' or '@'. Location: Microsoft.AppConfiguration/preview/2022-11-01-preview/appconfiguration.json#L256
⚠️ SuccessResponseBody	All success responses except 202 & 204 should define a response body. Location: Microsoft.AppConfiguration/preview/2022-11-01-preview/appconfiguration.json#L267
⚠️ ErrorResponse	Error response should have a schema. Location: Microsoft.AppConfiguration/preview/2022-11-01-preview/appconfiguration.json#L276

️️✔️Avocado succeeded [Detail] [Expand]

Validation passes for Avocado.

️️✔️ApiReadinessCheck succeeded [Detail] [Expand]

️⚠️~[Staging] ServiceAPIReadinessTest: 0 Warnings warning [Detail]

API Test is not triggered due to precheck failure. Check pipeline log for details.

️️✔️SwaggerAPIView succeeded [Detail] [Expand]

️️✔️CadlAPIView 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).

️️✔️CadlValidation succeeded [Detail] [Expand]

Validation passes for CadlValidation.

️️✔️PR Summary succeeded [Detail] [Expand]

Validation passes for Summary.

_{Posted by Swagger Pipeline | How to fix these errors?}

openapi-pipeline-app · 2023-03-16T04:00:15Z

Swagger pipeline restarted successfully, please wait for status update in this comment.

openapi-pipeline-app · 2023-03-16T04:00:15Z

Generated ApiView

Language	Package Name	ApiView Link
swagger	Microsoft.AppConfiguration	https://apiview.dev/Assemblies/Review/50ce1515caa64510841d09cc464defa4

...ation/data-plane/Microsoft.AppConfiguration/preview/2022-11-01-preview/appconfiguration.json

…ration/preview/2022-11-01-preview/appconfiguration.json

...ation/data-plane/Microsoft.AppConfiguration/preview/2022-11-01-preview/appconfiguration.json

openapi-workflow-bot · 2023-03-16T04:07:08Z

NewApiVersionRequired reason:

A service’s API is a contract with customers and is represented by using the api-version query parameter. Changes such as adding an optional property to a request/response or introducing a new operation is a change to the service’s contract and therefore requires a new api-version value. This is critically important for documentation, client libraries, and customer support.

EXAMPLE: if a customer calls a service in the public cloud using api-version=2020-07-27, the new property or operation may exist but if they call the service in a government cloud, air-gapped cloud, or Azure Stack Hub cloud using the same api-version, the property or operation may not exist. Because there is no clear relationship between the service api-version and the new property/operation, customers can’t trust the documentation and Azure customer have difficulty helping customers diagnose issues. In addition, each client library version documents the service version it supports. When an optional property or new operation is added to a service and its Swagger, new client libraries must be produced to expose this functionality to customers. Without updating the api-version, it is unclear to customers which version of a client library supports these new features.

...ation/data-plane/Microsoft.AppConfiguration/preview/2022-11-01-preview/appconfiguration.json

deyaaeldeen · 2023-03-16T04:30:07Z

...ation/data-plane/Microsoft.AppConfiguration/preview/2022-11-01-preview/appconfiguration.json

        }
      }
    },
+    "/operations/": {


I understand that this is the path the API currently uses for polling URLs but I wonder if we should make it more specific (e.g. /operations/snapshot/create or something) to future-proof it in case other long-running operations are added.

This is the URL right now.
https://endpoint.io/operations?api-version=2022-11-01-preview\u0026snapshot=snapshot-1678937759509

Service has to change if we want to be more specific here.

Yes, this is feedback to the service team to consider. /cc @JeffreyRichter

LRO status is generic resource. In such case our thought is that the design of the endpoint should be generic as well.

FWIW, I didn't see a one-size-fits-all endpoint for status monitoring like this before so I wanted to make sure that future-proofing is taken into consideration. Even something like /snapshots/operations feels much safer to me. That being said, I defer to the architects on this @tg-msft @johanste @JeffreyRichter

Our data-plane guidance is here: https://github.com/microsoft/api-guidelines/blob/vNext/azure/Guidelines.md#long-running-operations--jobs. The URL for the operation COLLECTION is not LISTable and so there is no polymorphism issue. Customers can only GET a single operation and they know its type.

I'm not sure I'm addressing your question but I think I am.

...ation/data-plane/Microsoft.AppConfiguration/preview/2022-11-01-preview/appconfiguration.json

…ration/preview/2022-11-01-preview/appconfiguration.json Co-authored-by: Deyaaeldeen Almahallawi <dealmaha@microsoft.com>

…ration/preview/2022-11-01-preview/appconfiguration.json

openapi-workflow-bot · 2023-03-16T23:43:37Z

Hi @HarshaNalluru, Your PR has some issues. Please fix the CI sequentially by following the order of Avocado, semantic validation, model validation, breaking change, lintDiff. If you have any questions, please post your questions in this channel https://aka.ms/swaggersupport.

Task	How to fix	Priority
Avocado	Fix-Avocado	High
Semantic validation	Fix-SemanticValidation-Error	High
Model validation	Fix-ModelValidation-Error	High
LintDiff	Fix-LintDiff	high

If you need further help, please feedback via swagger feedback.

...plane/Microsoft.AppConfiguration/preview/2022-11-01-preview/examples/GetOperationStatus.json

…ration/preview/2022-11-01-preview/examples/GetOperationStatus.json Co-authored-by: Deyaaeldeen Almahallawi <dealmaha@microsoft.com>

deyaaeldeen

LGTM. I have one comment unresolved and it is directed to the service team. Could you please share it with them? Thanks.

jimmyca15 · 2023-03-17T18:43:50Z

...ation/data-plane/Microsoft.AppConfiguration/preview/2022-11-01-preview/appconfiguration.json

+        },
+        "error": {
+          "description": "An error, available when the status is `Failed`, describing why the operation failed.",
+          "$ref": "#/definitions/Error"          


This isn't the correct error schema for LRO.

Should be the ErrorDetail object mentioned here

@HarshaNalluru I have updated the spec to use the ErrorDetail schema defined in the Azure guidelines. could you take another look?

Looks good @jimmyca15, thank you!

…API guidelines.

mikekistler

Looks good! 👍

…23125) * Update appconfiguration.json * Update specification/appconfiguration/data-plane/Microsoft.AppConfiguration/preview/2022-11-01-preview/appconfiguration.json * Update specification/appconfiguration/data-plane/Microsoft.AppConfiguration/preview/2022-11-01-preview/appconfiguration.json Co-authored-by: Deyaaeldeen Almahallawi <dealmaha@microsoft.com> * Update specification/appconfiguration/data-plane/Microsoft.AppConfiguration/preview/2022-11-01-preview/appconfiguration.json * Update appconfiguration.json - Deya feedback * example GetOperationStatus * Update specification/appconfiguration/data-plane/Microsoft.AppConfiguration/preview/2022-11-01-preview/examples/GetOperationStatus.json Co-authored-by: Deyaaeldeen Almahallawi <dealmaha@microsoft.com> * Updates to descriptions, names, id format. * Update operation status to use error detail as defined by Azure REST API guidelines. * Prettier fixes. * dummy commit for CI --------- Co-authored-by: Deyaaeldeen Almahallawi <dealmaha@microsoft.com> Co-authored-by: Jimmy Campbell <jimmyca@microsoft.com>

Update appconfiguration.json

4ac157a

HarshaNalluru commented Mar 16, 2023

View reviewed changes

...ation/data-plane/Microsoft.AppConfiguration/preview/2022-11-01-preview/appconfiguration.json Outdated Show resolved Hide resolved

Update specification/appconfiguration/data-plane/Microsoft.AppConfigu…

0346360

…ration/preview/2022-11-01-preview/appconfiguration.json

HarshaNalluru commented Mar 16, 2023

View reviewed changes

...ation/data-plane/Microsoft.AppConfiguration/preview/2022-11-01-preview/appconfiguration.json Outdated Show resolved Hide resolved

HarshaNalluru requested a review from deyaaeldeen March 16, 2023 04:07

AzureRestAPISpecReview added data-plane NewApiVersionRequired labels Mar 16, 2023

deyaaeldeen reviewed Mar 16, 2023

View reviewed changes

...ation/data-plane/Microsoft.AppConfiguration/preview/2022-11-01-preview/appconfiguration.json Outdated Show resolved Hide resolved

deyaaeldeen reviewed Mar 16, 2023

View reviewed changes

...ation/data-plane/Microsoft.AppConfiguration/preview/2022-11-01-preview/appconfiguration.json Outdated Show resolved Hide resolved

deyaaeldeen reviewed Mar 16, 2023

View reviewed changes

...ation/data-plane/Microsoft.AppConfiguration/preview/2022-11-01-preview/appconfiguration.json Outdated Show resolved Hide resolved

deyaaeldeen reviewed Mar 16, 2023

View reviewed changes

...ation/data-plane/Microsoft.AppConfiguration/preview/2022-11-01-preview/appconfiguration.json Outdated Show resolved Hide resolved

HarshaNalluru and others added 4 commits March 16, 2023 14:31

Update specification/appconfiguration/data-plane/Microsoft.AppConfigu…

9dffc37

…ration/preview/2022-11-01-preview/appconfiguration.json Co-authored-by: Deyaaeldeen Almahallawi <dealmaha@microsoft.com>

Update specification/appconfiguration/data-plane/Microsoft.AppConfigu…

c7ff8f5

…ration/preview/2022-11-01-preview/appconfiguration.json

Update appconfiguration.json - Deya feedback

d02d037

example GetOperationStatus

dd68f6d

HarshaNalluru marked this pull request as ready for review March 16, 2023 23:37

HarshaNalluru requested a review from a team as a code owner March 16, 2023 23:37

HarshaNalluru requested review from MushMal and tg-msft and removed request for a team March 16, 2023 23:37

openapi-assignment-bot bot assigned anuchandy Mar 16, 2023

HarshaNalluru requested review from deyaaeldeen and jimmyca15 March 16, 2023 23:40

AzureRestAPISpecReview added the CI-FixRequiredOnFailure label Mar 16, 2023

deyaaeldeen reviewed Mar 16, 2023

View reviewed changes

...plane/Microsoft.AppConfiguration/preview/2022-11-01-preview/examples/GetOperationStatus.json Outdated Show resolved Hide resolved

HarshaNalluru changed the title ~~Update appconfiguration.json~~ Update appconfiguration.json to include the "operations" path Mar 16, 2023

Update specification/appconfiguration/data-plane/Microsoft.AppConfigu…

3a1628b

…ration/preview/2022-11-01-preview/examples/GetOperationStatus.json Co-authored-by: Deyaaeldeen Almahallawi <dealmaha@microsoft.com>

deyaaeldeen approved these changes Mar 16, 2023

View reviewed changes

HarshaNalluru requested review from joheredi and jsquire March 17, 2023 00:48

Updates to descriptions, names, id format.

4ad0c46

jsquire approved these changes Mar 17, 2023

View reviewed changes

jimmyca15 approved these changes Mar 17, 2023

View reviewed changes

jimmyca15 reviewed Mar 17, 2023

View reviewed changes

jimmyca15 self-requested a review March 17, 2023 18:44

Update operation status to use error detail as defined by Azure REST …

eb4dd87

…API guidelines.

AzureRestAPISpecReview removed the CI-FixRequiredOnFailure label Mar 17, 2023

Prettier fixes.

6af89fd

HarshaNalluru removed the NewApiVersionRequired label Mar 22, 2023

dummy commit for CI

48125b4

AzureRestAPISpecReview added the NewApiVersionRequired label Mar 22, 2023

HarshaNalluru requested a review from mikekistler March 24, 2023 10:32

mikekistler approved these changes Mar 24, 2023

View reviewed changes

mikekistler added the Approved-BreakingChange DO NOT USE! OBSOLETE label. See https://github.com/Azure/azure-sdk-tools/issues/6374 label Mar 24, 2023

jsquire merged commit 2ac8dec into main Mar 24, 2023

jsquire deleted the HarshaNalluru-patch-1 branch March 24, 2023 18:37

Conversation

HarshaNalluru commented Mar 16, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Data Plane API - Pull Request

Uh oh!

openapi-workflow-bot bot commented Mar 16, 2023

Uh oh!

openapi-pipeline-app bot commented Mar 16, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Swagger Validation Report

Uh oh!

openapi-pipeline-app bot commented Mar 16, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openapi-pipeline-app bot commented Mar 16, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Generated ApiView

Uh oh!

Uh oh!

Uh oh!

openapi-workflow-bot bot commented Mar 16, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

deyaaeldeen Mar 16, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

HarshaNalluru Mar 16, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

openapi-workflow-bot bot commented Mar 16, 2023

Uh oh!

Uh oh!

deyaaeldeen left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mikekistler left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

HarshaNalluru commented Mar 16, 2023 •

edited

Loading

openapi-pipeline-app bot commented Mar 16, 2023 •

edited

Loading

openapi-pipeline-app bot commented Mar 16, 2023 •

edited

Loading

openapi-pipeline-app bot commented Mar 16, 2023 •

edited

Loading

deyaaeldeen Mar 16, 2023 •

edited

Loading

HarshaNalluru Mar 16, 2023 •

edited

Loading