Skip to content

Conversation

@jecmenicanikola
Copy link
Contributor

@jecmenicanikola jecmenicanikola commented Apr 3, 2024

While investigating Azure/azure-sdk-for-net#39869 and Azure/azure-sdk-for-net#38719, I found that the type returned by the server and the type that the server expects does not match what is currently on the spec.

This PR changes the type of 'sectionType' property from string to array and the type of 'roundaboutExitNumber' property from string to int.

When the SDK code is regenerated with these changes, custom code will have to be adapted to the new types.

@jecmenicanikola jecmenicanikola requested a review from a team as a code owner April 3, 2024 14:53
@jecmenicanikola jecmenicanikola requested review from bexxx and mikekistler and removed request for a team April 3, 2024 14:53
@openapi-pipeline-app
Copy link

openapi-pipeline-app bot commented Apr 3, 2024

Next Steps to Merge

Important checks have failed. As of today they are not blocking this PR, but in near future they will.
Addressing the following failures is highly recommended:
  • ⚠️ The check named TypeSpec Requirement (data-plane) has failed. TypeSpec usage is required for all new (greenfield) services. This is currently enforced as a warning for data-plane specs, but will be made a blocking error in the near future. For information on converting from OpenAPI specs to TypeSpec specs or on data-plane (DP) policies, refer to aka.ms/azsdk/typespec. If you have general questions on resource provider (RP) policies, refer to aka.ms/rphelp
If you still want to proceed merging this PR without addressing the above failures, refer to step 4 in the PR workflow diagram.

@openapi-pipeline-app
Copy link

openapi-pipeline-app bot commented Apr 3, 2024

Swagger Validation Report

️❌BreakingChange: 17 Errors, 0 Warnings failed [Detail]
Compared specs (v0.10.9) new version base version
route.json 1.0(eb7a854) 1.0(main)
Rule Message
1023 - TypeFormatChanged The new version has a different format 'int64' than the previous one ''.
New: Route/preview/1.0/route.json#L2218:9
Old: Route/preview/1.0/route.json#L2214:9
1026 - TypeChanged The new version has a different type 'array' than the previous one 'string'.
New: Route/preview/1.0/route.json#L902:9
Old: Route/preview/1.0/route.json#L898:9
1026 - TypeChanged The new version has a different type 'array' than the previous one 'string'.
New: Route/preview/1.0/route.json#L1045:9
Old: Route/preview/1.0/route.json#L1041:9
1026 - TypeChanged The new version has a different type 'array' than the previous one 'string'.
New: Route/preview/1.0/route.json#L1152:9
Old: Route/preview/1.0/route.json#L1148:9
1026 - TypeChanged The new version has a different type 'integer' than the previous one 'string'.
New: Route/preview/1.0/route.json#L2218:9
Old: Route/preview/1.0/route.json#L2214:9
1026 - TypeChanged The new version has a different type 'array' than the previous one 'string'.
New: Route/preview/1.0/route.json#L1325:9
Old: Route/preview/1.0/route.json#L1321:9
1026 - TypeChanged The new version has a different type 'array' than the previous one 'string'.
New: Route/preview/1.0/route.json#L609:5
Old: Route/preview/1.0/route.json#L609:5
1028 - ArrayCollectionFormatChanged The new version has a different array collection format than the previous one.
New: Route/preview/1.0/route.json#L902:9
Old: Route/preview/1.0/route.json#L898:9
1028 - ArrayCollectionFormatChanged The new version has a different array collection format than the previous one.
New: Route/preview/1.0/route.json#L1045:9
Old: Route/preview/1.0/route.json#L1041:9
1028 - ArrayCollectionFormatChanged The new version has a different array collection format than the previous one.
New: Route/preview/1.0/route.json#L1152:9
Old: Route/preview/1.0/route.json#L1148:9
1028 - ArrayCollectionFormatChanged The new version has a different array collection format than the previous one.
New: Route/preview/1.0/route.json#L1325:9
Old: Route/preview/1.0/route.json#L1321:9
1028 - ArrayCollectionFormatChanged The new version has a different array collection format than the previous one.
New: Route/preview/1.0/route.json#L609:5
Old: Route/preview/1.0/route.json#L609:5
1049 - RemovedXmsEnum The new version is missing a 'x-ms-enum' found in the old version.
Old: Route/preview/1.0/route.json#L898:9
1049 - RemovedXmsEnum The new version is missing a 'x-ms-enum' found in the old version.
Old: Route/preview/1.0/route.json#L1041:9
1049 - RemovedXmsEnum The new version is missing a 'x-ms-enum' found in the old version.
Old: Route/preview/1.0/route.json#L1148:9
1049 - RemovedXmsEnum The new version is missing a 'x-ms-enum' found in the old version.
Old: Route/preview/1.0/route.json#L1321:9
1049 - RemovedXmsEnum The new version is missing a 'x-ms-enum' found in the old version.
New: Route/preview/1.0/route.json#L609:5
Old: Route/preview/1.0/route.json#L609:5
️️✔️Breaking Change(Cross-Version) succeeded [Detail] [Expand]
There are no breaking changes.
️️✔️CredScan succeeded [Detail] [Expand]
There is no credential detected.
️⚠️LintDiff: 0 Warnings warning [Detail]
Compared specs (v2.2.2) new version base version
1.0 1.0(eb7a854) 1.0(main)

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: Route/preview/1.0/route.json#L525
IntegerTypeMustHaveFormat The integer type does not have a format, please add it.
Location: Route/preview/1.0/route.json#L533
IntegerTypeMustHaveFormat The integer type does not have a format, please add it.
Location: Route/preview/1.0/route.json#L541
IntegerTypeMustHaveFormat The integer type does not have a format, please add it.
Location: Route/preview/1.0/route.json#L576
IntegerTypeMustHaveFormat The integer type does not have a format, please add it.
Location: Route/preview/1.0/route.json#L766
OperationIdNounVerb Per the Noun_Verb convention for Operation Ids, the noun 'Route' should not appear after the underscore. Note: If you have already shipped an SDK on top of this spec, fixing this warning may introduce a breaking change.
Location: Route/preview/1.0/route.json#L891
OperationIdNounVerb Per the Noun_Verb convention for Operation Ids, the noun 'Route' should not appear after the underscore. Note: If you have already shipped an SDK on top of this spec, fixing this warning may introduce a breaking change.
Location: Route/preview/1.0/route.json#L997
OperationIdNounVerb Per the Noun_Verb convention for Operation Ids, the noun 'Route' should not appear after the underscore. Note: If you have already shipped an SDK on top of this spec, fixing this warning may introduce a breaking change.
Location: Route/preview/1.0/route.json#L1038
OperationIdNounVerb Per the Noun_Verb convention for Operation Ids, the noun 'Route' should not appear after the underscore. Note: If you have already shipped an SDK on top of this spec, fixing this warning may introduce a breaking change.
Location: Route/preview/1.0/route.json#L1146
IntegerTypeMustHaveFormat The integer type does not have a format, please add it.
Location: Route/preview/1.0/route.json#L1165
IntegerTypeMustHaveFormat The integer type does not have a format, please add it.
Location: Route/preview/1.0/route.json#L1176
IntegerTypeMustHaveFormat The integer type does not have a format, please add it.
Location: Route/preview/1.0/route.json#L1188
OperationIdNounVerb Per the Noun_Verb convention for Operation Ids, the noun 'Route' should not appear after the underscore. Note: If you have already shipped an SDK on top of this spec, fixing this warning may introduce a breaking change.
Location: Route/preview/1.0/route.json#L1318
IntegerTypeMustHaveFormat The integer type does not have a format, please add it.
Location: Route/preview/1.0/route.json#L1338
IntegerTypeMustHaveFormat The integer type does not have a format, please add it.
Location: Route/preview/1.0/route.json#L1349
IntegerTypeMustHaveFormat The integer type does not have a format, please add it.
Location: Route/preview/1.0/route.json#L1355
OperationIdNounVerb Per the Noun_Verb convention for Operation Ids, the noun 'Route' should not appear after the underscore. Note: If you have already shipped an SDK on top of this spec, fixing this warning may introduce a breaking change.
Location: Route/preview/1.0/route.json#L1502
OperationIdNounVerb Per the Noun_Verb convention for Operation Ids, the noun 'Route' should not appear after the underscore. Note: If you have already shipped an SDK on top of this spec, fixing this warning may introduce a breaking change.
Location: Route/preview/1.0/route.json#L1663
OperationIdNounVerb Per the Noun_Verb convention for Operation Ids, the noun 'Route' should not appear after the underscore. Note: If you have already shipped an SDK on top of this spec, fixing this warning may introduce a breaking change.
Location: Route/preview/1.0/route.json#L1712
OperationIdNounVerb Per the Noun_Verb convention for Operation Ids, the noun 'Route' should not appear after the underscore. Note: If you have already shipped an SDK on top of this spec, fixing this warning may introduce a breaking change.
Location: Route/preview/1.0/route.json#L1753
IntegerTypeMustHaveFormat The integer type does not have a format, please add it.
Location: Route/preview/1.0/route.json#L1892
IntegerTypeMustHaveFormat The integer type does not have a format, please add it.
Location: Route/preview/1.0/route.json#L1897
IntegerTypeMustHaveFormat The integer type does not have a format, please add it.
Location: Route/preview/1.0/route.json#L1902
IntegerTypeMustHaveFormat The integer type does not have a format, please add it.
Location: Route/preview/1.0/route.json#L1919
IntegerTypeMustHaveFormat The integer type does not have a format, please add it.
Location: Route/preview/1.0/route.json#L1924
IntegerTypeMustHaveFormat The integer type does not have a format, please add it.
Location: Route/preview/1.0/route.json#L1953
IntegerTypeMustHaveFormat The integer type does not have a format, please add it.
Location: Route/preview/1.0/route.json#L1958
IntegerTypeMustHaveFormat The integer type does not have a format, please add it.
Location: Route/preview/1.0/route.json#L1963
IntegerTypeMustHaveFormat The integer type does not have a format, please add it.
Location: Route/preview/1.0/route.json#L1974
IntegerTypeMustHaveFormat The integer type does not have a format, please add it.
Location: Route/preview/1.0/route.json#L1979
️️✔️Avocado succeeded [Detail] [Expand]
Validation passes for Avocado.
️️✔️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.
️️✔️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]
Posted by Swagger Pipeline | How to fix these errors?

@openapi-pipeline-app
Copy link

openapi-pipeline-app bot commented Apr 3, 2024

Swagger Generation Artifacts

️️✔️ApiDocPreview succeeded [Detail] [Expand]
 Please click here to preview with your @microsoft account. 
Posted by Swagger Pipeline | How to fix these errors?

@openapi-pipeline-app
Copy link

openapi-pipeline-app bot commented Apr 3, 2024

Generated ApiView

Language Package Name ApiView Link
Swagger 1.0 https://apiview.dev/Assemblies/Review/84982204ccf042a393dc551a152f6a60?revisionId=508ea22a5c534ee7bc8195142753e9b7

@microsoft-github-policy-service microsoft-github-policy-service bot added the customer-reported Issues that are reported by GitHub users external to the Azure organization. label Apr 3, 2024
@microsoft-github-policy-service
Copy link
Contributor

Thank you for your contribution jecmenicanikola! We will review the pull request and get back to you soon.

@AzureRestAPISpecReview AzureRestAPISpecReview added BreakingChangeReviewRequired <valid label in PR review process>add this label when breaking change review is required data-plane labels Apr 3, 2024
@dubiety
Copy link
Member

dubiety commented Apr 17, 2024

Hi @mikekistler and @bexxx ,
The API owner PM @FarazGIS already checked the document is wrong. The current and expected behavior should be the same as the current PR. Can you help to approve the PR?

@dubiety dubiety requested a review from FarazGIS April 17, 2024 11:46
@dubiety
Copy link
Member

dubiety commented Apr 17, 2024

Also adding @FarazGIS as reviewer for awareness

@mikekistler mikekistler added the BreakingChange-Approved-BugFix Changes are to correct the REST API definition to correctly describe service behavior label Apr 17, 2024
Copy link
Member

@mikekistler mikekistler left a comment

Choose a reason for hiding this comment

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

Looks good. 👍

"description": "Sections of the route that are located within urban areas."
}
]
"description": "Specifies which of the section types is reported in the route response. <br><br>For example if sectionType = pedestrian the sections which are suited for pedestrians only are returned. Can be specified multiple times in one request, for example, '&sectionType=carTrain&sectionType=pedestrain&sectionType=motorway'. The default sectionType refers to the travelMode input. By default travelMode is set to car",
Copy link
Member

Choose a reason for hiding this comment

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

Typo: pedestrain -> pedestrian

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dubiety done

@dubiety dubiety merged commit eeb6a46 into Azure:main Jun 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

BreakingChange-Approved-BugFix Changes are to correct the REST API definition to correctly describe service behavior BreakingChangeReviewRequired <valid label in PR review process>add this label when breaking change review is required customer-reported Issues that are reported by GitHub users external to the Azure organization. data-plane

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants