[Search] Add client names for ResetSkills and ResetDocs by Mohit-Chakraborty · Pull Request #16222 · Azure/azure-rest-api-specs

Mohit-Chakraborty · 2021-09-29T20:38:54Z

Without any name provided in the spec, CodeGen sets names like "Paths1Ju2XepSkillsetsSkillsetnameSearchResetskillsPostRequestbodyContentApplicationJsonSchema" and "Paths1Cj7DxmIndexersIndexernameSearchResetdocsPostRequestbodyContentApplicationJsonSchema"

openapi-workflow-bot · 2021-09-29T20:38:55Z

Hi, @Mohit-Chakraborty 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. vsswagger@microsoft.com

openapi-pipeline-app · 2021-09-29T20:38:56Z

Swagger Validation Report

️⚠️BreakingChange: 1 Warnings warning [Detail]

Compared Swaggers (Based on Oad v0.9.0)
- current:preview/2021-04-30-Preview/searchservice.json compared with base:preview/2021-04-30-Preview/searchservice.json

Rule	Message
⚠️ 1026 - TypeChanged	The new version has a different type 'object' than the previous one ''. New: Azure.Search/preview/2021-04-30-Preview/searchservice.json#L1064:13 Old: Azure.Search/preview/2021-04-30-Preview/searchservice.json#L1063:13

️⚠️LintDiff: 0 Warnings warning [Detail]

Linted configuring files (Based on source branch, openapi-validator v1.10.1 , classic-openapi-validator v1.1.10 )
- data-plane/Azure.Search/readme.md tag:package-2021-04-searchservice-preview
Linted configuring files (Based on target branch, openapi-validator v1.10.1 , classic-openapi-validator v1.1.10 )
- data-plane/Azure.Search/readme.md tag:package-2021-04-searchservice-preview

The following errors/warnings exist before current PR submission:

Rule	Message
⚠️ R2007 - LongRunningOperationsWithLongRunningExtension	The operation 'Indexers_Run' returns 202 status code, which indicates a long running operation, please enable 'x-ms-long-running-operation. Location: Azure.Search/preview/2021-04-30-Preview/searchservice.json#L413
⚠️ R4000 - ParameterDescriptionRequired	'keysOrIds' parameter lacks 'description' property. Consider adding a 'description' element. Accurate description is essential for maintaining reference documentation. Location: Azure.Search/preview/2021-04-30-Preview/searchservice.json#L358

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

Validation passes for Avocado.

️️✔️ModelValidation succeeded [Detail] [Expand]

Validation passes for ModelValidation.

️️✔️SemanticValidation succeeded [Detail] [Expand]

Validation passes for SemanticValidation.

️️✔️Cross-Version Breaking Changes succeeded [Detail] [Expand]

There are no breaking changes.

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

There is no credential detected.

️️✔️[Staging] SDK Track2 Validation succeeded [Detail] [Expand]

Validation passes for SDKTrack2Validation

The following tags are being changed in this PR

search/data-plane/Azure.Search/readme.md#package-2021-04-searchservice-preview

️️✔️[Staging] PrettierCheck succeeded [Detail] [Expand]

Validation passes for PrettierCheck.

️️✔️[Staging] SpellCheck succeeded [Detail] [Expand]

Validation passes for SpellCheck.

️️✔️[Staging] Lint(RPaaS) succeeded [Detail] [Expand]

Validation passes for Lint(RPaaS).

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

openapi-workflow-bot · 2021-09-29T20:38:56Z

[Call for Action] To better understand Azure service dev/test scenario, and support Azure service developer better on Swagger and REST API related tests in early phase, please help to fill in with this survey https://aka.ms/SurveyForEarlyPhase. It will take 5 to 10 minutes. If you already complete survey, please neglect this comment. Thanks.

openapi-pipeline-app · 2021-09-29T20:38:56Z

Swagger Generation Artifacts

️️✔️[Staging] ApiDocPreview succeeded [Detail] [Expand]

 Please click here to preview with your @microsoft account.

️️✔️[Staging] SDK Breaking Change Tracking succeeded [Detail] [Expand]

Breaking Changes Tracking

️❌ azure-sdk-for-net-track2 failed [Detail]

❌Failed [Logs]Release - Generate from 4e3ae66. SDK Automation 14.0.0

command	pwsh ./eng/scripts/Automation-Sdk-Init.ps1 ../azure-sdk-for-net_tmp/initInput.json ../azure-sdk-for-net_tmp/initOutput.json
warn	File azure-sdk-for-net_tmp/initOutput.json not found to read
command	pwsh ./eng/scripts/Automation-Sdk-Generate.ps1 ../azure-sdk-for-net_tmp/generateInput.json ../azure-sdk-for-net_tmp/generateOutput.json
cmderr	[Automation-Sdk-Generate.ps1] �[91mWrite-Error: �[91mError occurred while generating artifacts for Azure.Search.Documents
cmderr	[Automation-Sdk-Generate.ps1] �[0m

❌Azure.Search.Documents [View full logs] [Release SDK Changes]
```
info	[Changelog]
```

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

openapi-workflow-bot · 2021-09-29T20:42:32Z

Hi @Mohit-Chakraborty, Your PR has some issues. Please fix the CI sequentially by following the order of Avocado, semantic validation, model validation, breaking change, lintDiff.

Task	How to fix	Priority	Support (Microsoft alias)
Avocado	Fix-Avocado	High	ruowan
Semantic validation	Fix-SemanticValidation-Error	High	raychen, jianyxi
Model validation	Fix-ModelValidation-Error	High	raychen,jianyxi
LintDiff	Fix-LintDiff	high	jianyxi, ruoxuan

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

Mohit-Chakraborty · 2021-09-29T20:42:45Z

specification/search/data-plane/Azure.Search/preview/2021-04-30-Preview/searchservice.json

@@ -1061,6 +1062,8 @@
            "in": "body",
            "required": true,
            "schema": {


Still not happy with this part.
This generates a class with an IList as its only property -

public partial class SkillNames { /// <summary> Initializes a new instance of SkillNames. </summary> public SkillNames() { SkillNamesValue = new ChangeTrackingList<string>(); } /// <summary> the names of skills to be reset. </summary> public IList<string> SkillNamesValue { get; } }

Looking for ways to have this parameter as a list itself. Can we do that without breaking the service protocol?

I agree it doesn't seem to warrant its own type. @shuyangmsft ?

@Mohit-Chakraborty If you want to add skillNames as a parameter directly. You can add skillNames parameter like-

{ "name": "skillNames", "in": "body", "required": true, "schema": { "type": "array", "items": { "type": "string" } }, "description": "The names of skills to reset." }

Thanks @ShivangiReja. Do you know if this will be a breaking change for the service?

❌ 1033 - RemovedProperty The new version is missing a property found in the old version. Was 'skillNames' renamed or removed?Old: Azure.Search/preview/2021-04-30-Preview/searchservice.json#L1064:15

Validation tells me this is a breaking change.

So, we should probably make this change in the client to make things look better.

It looks like a breaking change, since previously user was passing the object type in the method and now with this change user will have to pass the list.

I reverted the change.

I still have an error -

❌ INVALID_TYPE Expected type object but found type arrayUrl: Azure.Search/preview/2021-04-30-Preview/searchservice.json#L1064

Finally managed to fix all validation errors. The above error was due to a discrepancy between the spec and the example for the ResetSkills operation. I updated the example to follow the design of the spec.

openapi-workflow-bot · 2021-09-29T21:38:57Z

Hi @Mohit-Chakraborty, one or multiple breaking change(s) is detected in your PR. Please check out the breaking change(s), and provide business justification in the PR comment and @ PR assignee why you must have these change(s), and how external customer impact can be mitigated. Please ensure to follow breaking change policy to request breaking change review and approval before proceeding swagger PR review.
Action: To initiate an evaluation of the breaking change, create a new intake using the template for breaking changes. Addition details on the process and office hours are on the Breaking change Wiki.
If you want to know the production traffic statistic, please see ARM Traffic statistic.
If you think it is false positive breaking change, please provide the reasons in the PR comment, report to Swagger Tooling Team via https://aka.ms/swaggerfeedback.

Mohit-Chakraborty · 2021-09-29T21:45:54Z

cc: @tg-msft

… property" This reverts commit 439d1ae.

This reverts commit f2896cd.

Mohit-Chakraborty · 2021-09-30T03:01:07Z

@anuchandy, @lmazuel can we merge this PR please.

* Add client names for ResetSkills and ResetDocs * Make 'skillNames' a list of items and not a class with a list property * Revert "Make 'skillNames' a list of items and not a class with a list property" This reverts commit 439d1ae. * Remove outer 'description' property * Revert "Remove outer 'description' property" This reverts commit f2896cd. * Try to fix the example as per the spec

Add client names for ResetSkills and ResetDocs

56692da

Mohit-Chakraborty requested review from AlexGhiondea, arv100kri, bleroy and brjohnstmsft as code owners September 29, 2021 20:38

openapi-assignment-bot bot assigned anuchandy Sep 29, 2021

Mohit-Chakraborty requested review from shuyangmsft and tg-msft September 29, 2021 20:39

openapi-pipeline-app bot added the CI-FixRequiredOnFailure label Sep 29, 2021

Mohit-Chakraborty commented Sep 29, 2021

View reviewed changes

Mohit-Chakraborty requested a review from ShivangiReja September 29, 2021 20:47

openapi-sdkautomation bot mentioned this pull request Sep 29, 2021

[AutoPR Azure.Search.Documents] [Search] Add client names for ResetSkills and ResetDocs AzureSDKAutomation/azure-sdk-for-net#6105

Closed

Make 'skillNames' a list of items and not a class with a list property

439d1ae

openapi-pipeline-app bot added the BreakingChangeReviewRequired label Sep 29, 2021

Revert "Make 'skillNames' a list of items and not a class with a list…

6731185

… property" This reverts commit 439d1ae.

openapi-pipeline-app bot removed the BreakingChangeReviewRequired label Sep 29, 2021

Mohit-Chakraborty added 3 commits September 29, 2021 16:15

Remove outer 'description' property

f2896cd

Revert "Remove outer 'description' property"

f996c16

This reverts commit f2896cd.

Try to fix the example as per the spec

8e1a301

openapi-pipeline-app bot removed the CI-FixRequiredOnFailure label Sep 30, 2021

bleroy approved these changes Sep 30, 2021

View reviewed changes

lmazuel approved these changes Sep 30, 2021

View reviewed changes

lmazuel merged commit 4e3ae66 into main Sep 30, 2021

openapi-sdkautomation bot mentioned this pull request Sep 30, 2021

[ReleasePR Azure.Search.Documents] Add aliases to 2021-04-30-preview swagger for Azure search Azure/azure-sdk-for-net#23725

Closed

lmazuel deleted the mohitc/AddClientNamesForCodeGen branch September 30, 2021 22:06

Conversation

Mohit-Chakraborty commented Sep 29, 2021

Uh oh!

openapi-workflow-bot bot commented Sep 29, 2021

Uh oh!

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

Uh oh!

Swagger Validation Report

Uh oh!

openapi-workflow-bot bot commented Sep 29, 2021

Uh oh!

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

Uh oh!

Swagger Generation Artifacts

Breaking Changes Tracking

Uh oh!

openapi-workflow-bot bot commented Sep 29, 2021

Uh oh!

Mohit-Chakraborty Sep 29, 2021 • 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!

Mohit-Chakraborty Sep 29, 2021 • 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!

openapi-workflow-bot bot commented Sep 29, 2021

Uh oh!

Mohit-Chakraborty commented Sep 29, 2021

Uh oh!

Mohit-Chakraborty commented Sep 30, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

openapi-pipeline-app bot commented Sep 29, 2021 •

edited

Loading

openapi-pipeline-app bot commented Sep 29, 2021 •

edited

Loading

Mohit-Chakraborty Sep 29, 2021 •

edited

Loading

Mohit-Chakraborty Sep 29, 2021 •

edited

Loading