Azure / azure-sdk-for-js

This repository is for active development of the Azure SDK for JavaScript (NodeJS & Browser). For consumers of the SDK we recommend visiting our public developer docs at https://docs.microsoft.com/javascript/azure/ or our versioned developer docs at https://azure.github.io/azure-sdk-for-js.
MIT License
2.07k stars 1.19k forks source link

[Cognitive Services] Review codesnippets in README #4289

Closed ramya-rao-a closed 5 years ago

ramya-rao-a commented 5 years ago

The code-snippets in README for libraries that are auto-generated have also been auto-generated. This issue is to review these code-snippets and to ensure that they are accurate among the data plane libraries for Cognitive services

Related to #4291

ramya-rao-a commented 5 years ago

Corrections to the client constructor in the samples:

Compile errors in typescript sample code

cc @daviwil fyi @amarzavery

ramya-rao-a commented 5 years ago

@daviwil, @amarzavery

Are the below 2 cases a consequence of how the swagger spec was written or the result of using different flags/options when running the code generator? I am trying to see where the long term fix should go.

amarzavery commented 5 years ago

In ContentModerator swagger spec the ApiKey is defined in security and securityDefinitions whereas, in Prediction swagger spec the ApiKey is defined in global parameters section and it has been explicitly stated that x-ms-parameter-location: "client". (More info on "x-ms-parameter-location" extension over here. Adding the extension x-ms-parameter-location: "client" for global parameters is redundant, since that is what Autorest does any ways for global parameters).

Constructor takes apikey and no credentials

Since the generic version of Autorest is used, if you need credentials either use --addcredentials=true or have security defined in the swagger spec.

First argument to constructor is endpoint, second is creds. The order here is flipped

May be they enforced it by adding x-ms-parameter-location: "client" to both the parameters endpoint and apikey and endpoint is defined before apikey in the Prediction swagger spec (It is my theory, not sure about this. Please make changes to the spec and regenerate to verify).

ramya-rao-a commented 5 years ago

cc @v-jaswel because he has been involved in getting started scenarios for a couple of Cognitive Services cc @chrishmsft who can help with reaching out to individual service teams as needed

ramya-rao-a commented 5 years ago

Update: New versions of luis-runtime & luis-authoring libraries have been released with

sarangan12 commented 5 years ago

All Cognitive Services has been released with modified Code Snippets and regenerated code. CLosing this issue as there is no pending action item

ramya-rao-a commented 5 years ago

Thanks @sarangan12! This was a tall order and I am glad that all the cognitive services are up to date with working samples now :)

cc @ChrisHMSFT, @loarabia