[Cognitive Services] Update endpoint URL template for Face. #3490
Conversation
Automation for azure-sdk-for-pythonThe initial PR has been merged into your service PR: |
Automation for azure-sdk-for-nodeThe initial PR has been merged into your service PR: |
Automation for azure-sdk-for-rubyThe initial PR has been merged into your service PR: |
| @@ -8,7 +8,6 @@ nodejs: | |||
| package-name: azure-cognitiveservices-face | |||
| package-version: 1.0.0-preview | |||
| output-folder: $(node-sdks-folder)/lib/services/face | |||
| override-client-name: FaceAPIClient | |||
There was a problem hiding this comment.
Remove this as the class name now naturally ends with "Client"
Automation for azure-sdk-for-javaThe initial PR has been merged into your service PR: |
Automation for azure-sdk-for-goThe initial PR has been merged into your service PR: |
yangyuan
changed the title
| @@ -1,6 +1,6 @@ | |||
| { | |||
| "parameters": { | |||
| "AzureRegion": "westus", | |||
| "Endpoint": "{Endpoint}", | |||
|
LGTM 👍 |
| }, | ||
| "Endpoint": { | ||
| "name": "Endpoint", | ||
| "description": "Supported Cognitive Services endpoints (protocol and hostname, for example: https://westus.api.cognitive.microsoft.com).", |
There was a problem hiding this comment.
I am a little concern about the Endpoint setting. Previously, we also have Endpoint concept in Windows SDK [1] and many other places where the endpoint is the whole base url including the path /face/v1.0. I prefer we keep consistency and that is also more general since the path dose not need to be fixed. But if this is a team decision as explained in #3489 that we will keep the same style in all cognitive services, I am OK with it.
There was a problem hiding this comment.
Hi @huxuan, the decision is a compromise according to the feedback from multiple teams as well as the limitations of swagger. (In fact we have this change because we prefer consistency over back compatibility)
In this PR we changed the definition of Endpoint, we are planning to update Endpoint in documents and Azure Portal when all the teams agree with the change.
There was a problem hiding this comment.
@huxuan BTW feel free to bring up you concerns in the e-mail thread.
|
Others LGTM. |
|
Hi @jhendrixMSFT, I guess we can merge this |
Detail background and explainations in here: #3489 Cognitive Services URL template (endpoint)
Please hold for @huxuan or @lebronJ 's review.
PR information
api-versionin the path should match theapi-versionin the spec).Quality of Swagger