Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LUIS runtime changes #5424

Merged
merged 2 commits into from
Apr 8, 2019
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,52 @@
"version": "2.0"
},
"x-ms-parameterized-host": {
"hostTemplate": "{Endpoint}/luis/v2.0",
"useSchemePrefix": false,
"hostTemplate": "{AzureRegion}.api.cognitive.microsoft.{AzureCloud}/luis/v2.0",
Copy link
Member

Choose a reason for hiding this comment

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

This change will prevent the users from using customized domain/containers.
Please revert this change.

If you believe this change is a must, we can kickstart an email thread to discuss abt this.

Copy link
Contributor

Choose a reason for hiding this comment

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

@diberry has this been addressed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@nebadr @AmirGeorge Is this something you can address? I don't think I introduced this issue.

Copy link
Contributor

Choose a reason for hiding this comment

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

@dsgouda @diberry this change has been reverted in the second commit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@nebadr - should I close this PR?

Copy link
Contributor

Choose a reason for hiding this comment

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

@diberry We want to merge this PR. I meant the change has been reverted here in this PR but in the second commit (this change appears to be outdated).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@nebadr Do I need to pull something into this PR to fix or it is good to go as it is now?

Copy link
Contributor

Choose a reason for hiding this comment

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

@diberry No, it is good. Thanks.

"parameters": [
{
"$ref": "#/parameters/Endpoint"
"name": "AzureRegion",
"description": "Supported Azure regions for Cognitive Services endpoints",
"x-ms-parameter-location": "client",
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason these are supposed to be client level parameters?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@nebadr @AmirGeorge - client level params?

Copy link
Contributor

Choose a reason for hiding this comment

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

@tsuwandy for possible input.

Copy link
Contributor

Choose a reason for hiding this comment

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

@dsgouda We are not aware exactly why but is it causing problem?

Copy link
Contributor

@dsgouda dsgouda Apr 1, 2019

Choose a reason for hiding this comment

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

This is not necessarily a problem. We want to ensure your intent is to tie each client instance to a particular Azure region.
Please confirm the same.

Copy link
Contributor

Choose a reason for hiding this comment

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

@dsgouda Yes, this is our intent. Confirmed.

"required": true,
"type": "string",
"in": "path",
"x-ms-skip-url-encoding": true,
"x-ms-enum": {
"name": "AzureRegions",
"modelAsString": false
},
"enum": [
"westus",
"westeurope",
"southeastasia",
"eastus2",
"westcentralus",
"westus2",
"eastus",
"southcentralus",
"northeurope",
"eastasia",
"australiaeast",
"brazilsouth",
"virginia"
]
},
{
"name": "AzureCloud",
"description": "Supported Azure Clouds for Cognitive Services endpoints",
"x-ms-parameter-location": "client",
"required": true,
"type": "string",
"in": "path",
"x-ms-skip-url-encoding": true,
"x-ms-enum": {
"name": "AzureClouds",
"modelAsString": false
},
"enum": [
"com",
"us"
]
}
]
},
Expand All @@ -35,6 +76,7 @@
"name": "appId",
"in": "path",
"type": "string",
"format": "uuid",
"required": true,
"description": "The LUIS application ID (guid)."
},
Expand Down Expand Up @@ -74,7 +116,7 @@
{
"name": "bing-spell-check-subscription-key",
"in": "query",
"description": "The subscription key to use when enabling bing spell check",
"description": "The subscription key to use when enabling Bing spell check",
"type": "string"
},
{
Expand Down Expand Up @@ -115,6 +157,7 @@
"name": "appId",
"in": "path",
"type": "string",
"format": "uuid",
"required": true,
"description": "The LUIS application ID (Guid)."
},
Expand Down Expand Up @@ -155,7 +198,7 @@
{
"name": "bing-spell-check-subscription-key",
"in": "query",
"description": "The subscription key to use when enabling bing spell check",
"description": "The subscription key to use when enabling Bing spell check",
"type": "string"
},
{
Expand Down Expand Up @@ -327,7 +370,7 @@
},
"additionalProperties": {
"type": "object",
"description": "List of additional properties. E.g.: score and resolution values for pre-built LUIS entities."
"description": "List of additional properties. For example, score and resolution values for pre-built LUIS entities."
},
"required": [
"entity",
Expand Down Expand Up @@ -406,14 +449,5 @@
}
},
"parameters": {
"Endpoint": {
"name": "Endpoint",
"description": "Supported Cognitive Services endpoints (protocol and hostname, for example: https://westus.api.cognitive.microsoft.com).",
"x-ms-parameter-location": "client",
"required": true,
"type": "string",
"in": "path",
"x-ms-skip-url-encoding": true
}
}
}