[CLU] Adding x-ms-client-names properties for confidenceScore

[AhmedLeithy]

To ensure consistency with question answering, any mention of confidenceScore should be renamed to confidence in the SDK. Since the question answering SDK implements this using the swagger "x-ms-client-name" property, we would like to add it to the conversations swagger as well. This change should be considered after the release of the PuP later today.

Manually adding folks for review.

@heaths @ChongTang @annatisch

I've also caught the following bug:
The QuestionAnsweringTargetIntentResult references the 2021-07-15-preview question answering swagger in this line of code, which does not have the renames for the confidenceScore properties. We would ideally like this to reference the stable question answering swagger. Should we bring the stable question answering swagger to this branch and reference it? Or should we add a static reference to the swagger on the other branch?

[openapi-workflow-bot]

Hi, @AhmedLeithy 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-workflow-bot]

[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]

Swagger Validation Report

️️✔️BreakingChange succeeded [Detail] [Expand]

There are no breaking changes.

️️✔️LintDiff succeeded [Detail] [Expand]

Validation passes for LintDiff.

• Linted configuring files (Based on source branch, openapi-validator v1.10.1 , classic-openapi-validator v1.1.10 )
  • data-plane/Language/readme.md tag:release_2021_11_01_preview
• Linted configuring files (Based on target branch, openapi-validator v1.10.1 , classic-openapi-validator v1.1.10 )
  • data-plane/Language/readme.md tag:release_2021_11_01_preview

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

️🔄[Staging] SDK Track2 Validation inProgress [Detail]

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

Validation passes for PrettierCheck.

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

Validation passes for SpellCheck.

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

[openapi-pipeline-app]

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 warning [Detail]

• ⚠️Warning [Logs] Generate from ae1a2a38a83222dffbc30c25e419ea2b5693efc7. SDK Automation 14.0.0

  warn	Skip initScript due to not configured
  command	sudo apt-get install -y dotnet-sdk-5.0
  command	autorest --version=V2 --csharp --reflect-api-versions --license-header=MICROSOFT_MIT_NO_VERSION --use=@microsoft.azure/autorest.csharp@2.3.82 --csharp-sdks-folder=/home/vsts/work/1/s/azure-sdk-for-net/sdk ../azure-rest-api-specs/specification/cognitiveservices/data-plane/Language/readme.md
  cmderr	[Autorest] realpath(): Permission denied
  cmderr	[Autorest] realpath(): Permission denied
  cmderr	[Autorest] realpath(): Permission denied
  cmderr	[Autorest] realpath(): Permission denied
  cmderr	[Autorest] realpath(): Permission denied
  cmderr	[Autorest] realpath(): Permission denied
  warn	No file changes detected after generation
  warn	No package detected after generation

️⚠️ azure-sdk-for-python warning [Detail]

• ⚠️Warning [Logs] Generate from ae1a2a38a83222dffbc30c25e419ea2b5693efc7. SDK Automation 14.0.0

  command	sh scripts/automation_init.sh ../azure-sdk-for-python_tmp/initInput.json ../azure-sdk-for-python_tmp/initOutput.json
  cmderr	[automation_init.sh] ERROR: pip's dependency resolver does not currently take into account all the packages that are installed. This behaviour is the source of the following dependency conflicts.
  cmderr	[automation_init.sh] azure-mgmt-core 1.3.0 requires azure-core<2.0.0,>=1.15.0, but you have azure-core 1.6.0 which is incompatible.
  cmderr	[automation_init.sh] ERROR: pip's dependency resolver does not currently take into account all the packages that are installed. This behaviour is the source of the following dependency conflicts.
  cmderr	[automation_init.sh] opencensus-ext-azure 1.1.0 requires azure-core<2.0.0,>=1.12.0, but you have azure-core 1.6.0 which is incompatible.
  cmderr	[automation_init.sh] azure-mgmt-core 1.3.0 requires azure-core<2.0.0,>=1.15.0, but you have azure-core 1.6.0 which is incompatible.
  cmderr	[automation_init.sh] azure-identity 1.7.0 requires azure-core<2.0.0,>=1.11.0, but you have azure-core 1.6.0 which is incompatible.
  cmderr	[automation_init.sh] WARNING: Skipping azure-nspkg as it is not installed.
  command	sh scripts/automation_generate.sh ../azure-sdk-for-python_tmp/generateInput.json ../azure-sdk-for-python_tmp/generateOutput.json
  warn	No file changes detected after generation
  warn	No package detected after generation

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

[heaths]

LGTM, but could we do any other renames while we're at it? Let me know and I can wait, or merge this now and you could follow-up with another PR (if there are any more renames in this swagger).

[ChongTang]

@AhmedLeithy The stable version of the Question Answering was not merged when I made this change. In the stable version, they removed the KnowledgeBaseAnswers definition and use the KnowledgeBaseAnswer as their response. Do you want me to create a PR to refer the stable version now? If I create a PR to update this, will it interrupt your work flow?

[AhmedLeithy]

@heaths
The other renames would be related to AnalyisParameters, listKeys, and NoneLinkedTargetIntentResult. I can do any of those changes in this PR, but if we still need to have a conversation about the new names, then it might make sense to merge this PR first, then create new issues for them later. (btw will merging this PR will have an effect on the CLU release pipeline?).

@ChongTang
Ah I see, I was not aware of the time difference. Creating a PR to update this would be much appreciated!

[ChongTang]

@AhmedLeithy Got it. I will create it soon. BTW. The renaming you are talking about happens in SDK right? We don't need to change the swagger. Correct?

[heaths]

@ChongTang you can safely change model names in the swagger as needed, but changing property names - if you don't have enough time in the service - can be done with x-ms-client-name that only impacts autorest-generated clients like the Azure SDKs.

[ChongTang]

Got it. For the confidence thing, we agreed to use "confidenceScore" instead of "score". I am not sure how "confidence" appears. :)

…

On Wed, Nov 3, 2021 at 10:39 AM Heath Stewart ***@***.***> wrote: @ChongTang <https://github.com/ChongTang> you can safely change model names in the swagger as needed, but changing property names - if you don't have enough time in the service - can be done with x-ms-client-name that only impacts autorest-generated clients like the Azure SDKs. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#16646 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AA3YR52GDDTIWA6QNYVGFDTUKF6VFANCNFSM5HIWWPKA> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.

[heaths]

Got it. For the confidence thing, we agreed to use "confidenceScore" instead of "score". I am not sure how "confidence" appears. :)

During the archboard review of Question Answering, the architects want just confidence or confidenceThreshold (instead of confidenceScoreThreshold) for the SDK. You do not need to change the model properties, but we'll want to be consistent across the SDKS.

[heaths]

@ChongTang, given this is what the SDK archboard wants, do you have any objections? You don't need to change the service models.

[ChongTang]

@ChongTang, given this is what the SDK archboard wants, do you have any objections? You don't need to change the service models.

Sounds good to me.