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

Update swagger to reconcile SDK generation mismatch #24670

Conversation

ericasp16
Copy link
Member

Data Plane API - Pull Request

The PhoneNumbers SDKs were generated on a swagger that is slightly different to the one that is checked into this repository. This is causing an issue when trying to make incremental changes for new features. This change aims to bring them into alignment and reach parity to unblock further SDK development

API Info: The Basics

Most of the information about your service should be captured in the issue that serves as your API Spec engagement record.

Is this review for (select one):

  • a private preview
  • a public preview
  • GA release

Change Scope

This section will help us focus on the specific parts of your API that are new or have been modified.
Please share a link to the design document for the new APIs, a link to the previous API Spec document (if applicable), and the root paths that have been updated.

❔Got questions? Need additional info?? We are here to help!

Contact us!

The Azure API Review Board is dedicated to helping you create amazing APIs. You can read about our mission and learn more about our process on our wiki.

Click here for links to tools, specs, guidelines & other good stuff

Tooling

Guidelines & Specifications

Helpful Links

@ericasp16 ericasp16 requested a review from a team as a code owner June 30, 2023 23:13
@ericasp16 ericasp16 requested review from stewartadam and weidongxu-microsoft and removed request for a team June 30, 2023 23:13
@openapi-workflow-bot
Copy link

Hi, @ericasp16! Thank you for your pull request. To help get your PR merged:
- Ensure you reviewed the checklists in the PR description. - Know that PR assignee is the person auto-assigned and responsible for your current PR review and approval. - 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.

@openapi-pipeline-app
Copy link

openapi-pipeline-app bot commented Jun 30, 2023

Swagger Validation Report

️❌BreakingChange: 1 Errors, 0 Warnings failed [Detail]
Rule Message
Runtime Exception "new":"https://github.com/Azure/azure-rest-api-specs/blob/ce93b56111bd5a3858557b59ee7b630c62c19078/specification/communication/data-plane/PhoneNumbers/stable/2022-12-01/phonenumbers.json",
"old":"https://github.com/Azure/azure-rest-api-specs/blob/main/specification/communication/data-plane/PhoneNumbers/stable/2022-12-01/phonenumbers.json",
"details":"Command failed: node "/mnt/vss/_work/_tasks/AzureApiValidation_5654d05d-82c1-48da-ad8f-161b817f6d41/0.0.54/common/temp/node_modules/.pnpm/@Azure[email protected]/node_modules/autorest/dist/app.js" --v2 --input-file=specification/communication/data-plane/PhoneNumbers/stable/2022-12-01/phonenumbers.json --output-artifact=swagger-document.json --output-artifact=swagger-document.map --output-file=new --output-folder=/tmp\nERROR: Syntax Error Encountered: Unexpected token in JSON\n - file:///mnt/vss/_work/1/azure-rest-api-specs/specification/communication/data-plane/PhoneNumbers/stable/2022-12-01/phonenumbers.json:1305:6\n"
️️✔️Breaking Change(Cross-Version) succeeded [Detail] [Expand]
There are no breaking changes.
️❌LintDiff: 2 Errors, 0 Warnings failed [Detail]
compared tags (via openapi-validator v2.1.3) new version base version
package-phonenumber-2022-12-01 package-phonenumber-2022-12-01(ce93b56) package-phonenumber-2022-12-01(ericasp/number-lookup-preview)

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

Rule Message Related RPC [For API reviewers]
XmsParameterLocation The parameter 'ApiVersionParameter' 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: PhoneNumbers/stable/2022-12-01/phonenumbers.json#L1392
HostParametersValidation The host parameter must be typed 'type 'string', format 'url''.
Location: PhoneNumbers/stable/2022-12-01/phonenumbers.json#L1424


The following errors/warnings exist before current PR submission:

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

Rule Message
⚠️ PathParameterSchema Path parameter should specify a maximum length (maxLength) and characters allowed (pattern).
Location: PhoneNumbers/stable/2022-12-01/phonenumbers.json#L20
⚠️ PaginationResponse Response body schema of pageable response should contain top-level array property value
Location: PhoneNumbers/stable/2022-12-01/phonenumbers.json#L97
⚠️ ErrorResponse Error response should contain a x-ms-error-code header.
Location: PhoneNumbers/stable/2022-12-01/phonenumbers.json#L101
⚠️ PaginationResponse Response body schema of pageable response should contain top-level array property value
Location: PhoneNumbers/stable/2022-12-01/phonenumbers.json#L159
⚠️ ErrorResponse Error response should contain a x-ms-error-code header.
Location: PhoneNumbers/stable/2022-12-01/phonenumbers.json#L163
⚠️ PathParameterSchema Path parameter should specify a maximum length (maxLength) and characters allowed (pattern).
Location: PhoneNumbers/stable/2022-12-01/phonenumbers.json#L192
⚠️ PaginationResponse Response body schema of pageable response should contain top-level array property value
Location: PhoneNumbers/stable/2022-12-01/phonenumbers.json#L234
⚠️ ErrorResponse Error response should contain a x-ms-error-code header.
Location: PhoneNumbers/stable/2022-12-01/phonenumbers.json#L238
⚠️ PathParameterSchema Path parameter should specify a maximum length (maxLength) and characters allowed (pattern).
Location: PhoneNumbers/stable/2022-12-01/phonenumbers.json#L267
⚠️ PaginationResponse Response body schema of pageable response should contain top-level array property value
Location: PhoneNumbers/stable/2022-12-01/phonenumbers.json#L331
⚠️ ErrorResponse Error response should contain a x-ms-error-code header.
Location: PhoneNumbers/stable/2022-12-01/phonenumbers.json#L335
⚠️ PathParameterSchema Path parameter should specify a maximum length (maxLength) and characters allowed (pattern).
Location: PhoneNumbers/stable/2022-12-01/phonenumbers.json#L367
⚠️ ErrorResponse Error response should contain a x-ms-error-code header.
Location: PhoneNumbers/stable/2022-12-01/phonenumbers.json#L412
⚠️ PathParameterSchema Path parameter should specify a maximum length (maxLength) and characters allowed (pattern).
Location: PhoneNumbers/stable/2022-12-01/phonenumbers.json#L441
⚠️ ErrorResponse Error response should contain a x-ms-error-code header.
Location: PhoneNumbers/stable/2022-12-01/phonenumbers.json#L459
⚠️ ErrorResponse Error response should contain a x-ms-error-code header.
Location: PhoneNumbers/stable/2022-12-01/phonenumbers.json#L515
⚠️ PathParameterSchema Path parameter should specify a maximum length (maxLength) and characters allowed (pattern).
Location: PhoneNumbers/stable/2022-12-01/phonenumbers.json#L544
⚠️ ErrorResponse Error response should contain a x-ms-error-code header.
Location: PhoneNumbers/stable/2022-12-01/phonenumbers.json#L568
⚠️ DeleteInOperationName 'DELETE' operation 'PhoneNumbers_CancelOperation' should use method name 'Delete'. Note: If you have already shipped an SDK on top of this spec, fixing this warning may introduce a breaking change.
Location: PhoneNumbers/stable/2022-12-01/phonenumbers.json#L586
⚠️ OperationId OperationId for delete method should contain 'Delete'
Location: PhoneNumbers/stable/2022-12-01/phonenumbers.json#L586
⚠️ PathParameterSchema Path parameter should specify a maximum length (maxLength) and characters allowed (pattern).
Location: PhoneNumbers/stable/2022-12-01/phonenumbers.json#L588
⚠️ ErrorResponse Error response should contain a x-ms-error-code header.
Location: PhoneNumbers/stable/2022-12-01/phonenumbers.json#L603
⚠️ PathParameterSchema Path parameter should specify a maximum length (maxLength) and characters allowed (pattern).
Location: PhoneNumbers/stable/2022-12-01/phonenumbers.json#L634
⚠️ RequestBodyOptional The body parameter is not marked as required.
Location: PhoneNumbers/stable/2022-12-01/phonenumbers.json#L644
⚠️ ErrorResponse Error response should contain a x-ms-error-code header.
Location: PhoneNumbers/stable/2022-12-01/phonenumbers.json#L678
⚠️ PathParameterSchema Path parameter should specify a maximum length (maxLength) and characters allowed (pattern).
Location: PhoneNumbers/stable/2022-12-01/phonenumbers.json#L707
⚠️ ErrorResponse Error response should contain a x-ms-error-code header.
Location: PhoneNumbers/stable/2022-12-01/phonenumbers.json#L725
⚠️ DeleteInOperationName 'DELETE' operation 'PhoneNumbers_ReleasePhoneNumber' should use method name 'Delete'. Note: If you have already shipped an SDK on top of this spec, fixing this warning may introduce a breaking change.
Location: PhoneNumbers/stable/2022-12-01/phonenumbers.json#L743
⚠️ OperationId OperationId for delete method should contain 'Delete'
Location: PhoneNumbers/stable/2022-12-01/phonenumbers.json#L743
⚠️ PathParameterSchema Path parameter should specify a maximum length (maxLength) and characters allowed (pattern).
Location: PhoneNumbers/stable/2022-12-01/phonenumbers.json#L745
️❌Avocado: 4 Errors, 0 Warnings failed [Detail]
Rule Message
JSON_PARSE The file is not a valid JSON file.
json: [PhoneNumbers/stable/2022-12-01/phonenumbers.json"}]({"kind":"structure","code":"unexpected token","position":{"line":1305,"column":7},"token":"}","message":"unexpected token, token: }, line: 1305, column: 7","url":"/mnt/vss/_work/1/c93b354fd9c14905bb574a8834c4d69b/specification/communication/data-plane/PhoneNumbers/stable/2022-12-01/phonenumbers.json"})
JSON_PARSE The file is not a valid JSON file.
json: [PhoneNumbers/stable/2022-12-01/phonenumbers.json"}]({"kind":"structure","code":"unexpected token","position":{"line":1306,"column":5},"token":"}","message":"unexpected token, token: }, line: 1306, column: 5","url":"/mnt/vss/_work/1/c93b354fd9c14905bb574a8834c4d69b/specification/communication/data-plane/PhoneNumbers/stable/2022-12-01/phonenumbers.json"})
JSON_PARSE The file is not a valid JSON file.
json: [PhoneNumbers/stable/2022-12-01/phonenumbers.json"}]({"kind":"structure","code":"unexpected token","position":{"line":1306,"column":6},"token":",","message":"unexpected token, token: ,, line: 1306, column: 6","url":"/mnt/vss/_work/1/c93b354fd9c14905bb574a8834c4d69b/specification/communication/data-plane/PhoneNumbers/stable/2022-12-01/phonenumbers.json"})
JSON_PARSE The file is not a valid JSON file.
json: [PhoneNumbers/stable/2022-12-01/phonenumbers.json"}]({"kind":"structure","code":"unexpected end of file","position":{"line":849,"column":18},"token":"}","message":"unexpected end of file, token: }, line: 849, column: 18","url":"/mnt/vss/_work/1/c93b354fd9c14905bb574a8834c4d69b/specification/communication/data-plane/PhoneNumbers/stable/2022-12-01/phonenumbers.json"})
️❌SwaggerAPIView: 0 Errors, 0 Warnings failed [Detail]
️️✔️CadlAPIView succeeded [Detail] [Expand]
️️✔️TypeSpecAPIView succeeded [Detail] [Expand]
️❌ModelValidation: 1 Errors, 0 Warnings failed [Detail]
Rule Message
JSON_PARSING_ERROR Json parsing error: unexpected token
Url: PhoneNumbers/stable/2022-12-01/phonenumbers.json#L1305:7
️❌SemanticValidation: 1 Errors, 0 Warnings failed [Detail]
Rule Message
JSON_PARSING_ERROR Json parsing error: unexpected token
JsonUrl: PhoneNumbers/stable/2022-12-01/phonenumbers.json#L1305:7
️❌PrettierCheck: 1 Errors, 0 Warnings failed [Detail]
Rule Message
HowToFix Code style issues found
path: PhoneNumbers/stable/2022-12-01/phonenumbers.json
️️✔️SpellCheck succeeded [Detail] [Expand]
Validation passes for SpellCheck.
️❌CadlValidation: 0 Errors, 0 Warnings failed [Detail]
️❌TypeSpec Validation: 0 Errors, 0 Warnings failed [Detail]
️️✔️PR Summary succeeded [Detail] [Expand]
Validation passes for Summary.
Posted by Swagger Pipeline | How to fix these errors?

@openapi-pipeline-app
Copy link

openapi-pipeline-app bot commented Jun 30, 2023

Swagger Generation Artifacts

️️✔️ApiDocPreview succeeded [Detail] [Expand]
 Please click here to preview with your @microsoft account. 
️️✔️SDK Breaking Change Tracking succeeded [Detail] [Expand]

Breaking Changes Tracking

Posted by Swagger Pipeline | How to fix these errors?

@openapi-pipeline-app
Copy link

openapi-pipeline-app bot commented Jun 30, 2023

Swagger pipeline restarted successfully. If there is ApiView generated, it will be updated in this comment.

@ericasp16 ericasp16 changed the base branch from main to ericasp/number-lookup-preview June 30, 2023 23:13
@openapi-workflow-bot
Copy link

Hi @ericasp16! Your PR has some issues. Please fix the CI issues, if present, in following order: Avocado, SemanticValidation, ModelValidation, Breaking Change, LintDiff.

TaskHow to fixPriority
AvocadoFix-AvocadoHigh
Semantic ValidationFix-SemanticValidation-ErrorHigh
Model ValidationFix-ModelValidation-ErrorHigh
LintDiffFix-LintDiffHigh

If you need further help, please reach out on the Teams channel aka.ms/azsdk/support/specreview-channel.

@openapi-workflow-bot
Copy link

Hi @ericasp16! The automation detected this pull request introduces changes to at least one existing API version that violate Azure's versioning policy. To comply with the policy, these changes must be made in a new API version. As a result, the automation added the NewApiVersionRequired label.

You cannot proceed with merging this PR until you complete one of the following action items:

ACTION ITEM ALTERNATIVE A: Introduce a new API version.
Submit a new PR instead of this one, or modify this PR, so that it adds a new API version instead of introducing changes to existing API versions.

ACTION ITEM ALTERNATIVE B: Request approval.
Alternatively, if you cannot avoid modifying existing API versions, then you can request an approval for them. Please follow the process described in the High-level Breaking Change Process doc.

ACTION ITEM ALTERNATIVE C: Report false positive.
If you think there are no changes in existing API version, i.e. there should be no NewApiVersionRequired label, then please explain why in a PR comment and @ the PR assignee.

For additional guidance, please see https://aka.ms/NewApiVersionRequired

@ericasp16 ericasp16 closed this Jul 1, 2023
@ericasp16
Copy link
Member Author

abandoning this review in favor of a new one: #24672

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants