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.

Sign up for GitHub

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

Jump to bottom

Adding mode flag to train command #1113

Merged
merged 3 commits into from
Feb 10, 2021
Merged

Adding mode flag to train command #1113

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8ea67bb
Adding mode flag to train command
munozemilio Feb 10, 2021
91ee8e3
Checking for error object
munozemilio Feb 10, 2021
5e214cd
Applying PR feedback
munozemilio Feb 10, 2021
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
3 changes: 2 additions & 1 deletion packages/luis/src/api/http-request.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,8 @@ const httpRequest = async function (subscriptionKey: string, config: any) {
if (error.response) {
// The request was made and the server responded with a status code
// that falls out of the range of 2xx
throw Error(error.response.statusText)
let message = error?.response?.data?.error?.message ?? ''
throw Error(`Code: ${error.response.statusText} ${message}`)
} else {
// Something happened in setting up the request that triggered an Error
throw Error(error.message)
Expand Down
7 changes: 5 additions & 2 deletions packages/luis/src/api/train.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,12 @@ const urlPath = '/luis/authoring/v3.0-preview/apps'
export default {
async train(
param: EndpointParameters,
versionId: string) {
versionId: string,
mode: string) {
let url = buildUrl(param.endpoint) + `/${param.appId}/versions/${versionId}/train`

if (mode) {
url += `?mode=${mode}`
}
return http.post(url, param.subscriptionKey, {})
},

Expand Down
5 changes: 3 additions & 2 deletions packages/luis/src/commands/luis/train/run.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ export default class LuisTrainRun extends Command {
subscriptionKey: flags.string({description: '(required) LUIS cognitive services subscription key (default: config:LUIS:subscriptionKey)'}),
appId: flags.string({description: '(required) LUIS application Id (defaults to config:LUIS:appId)'}),
versionId: flags.string({description: '(required) Version to show training status (defaults to config:LUIS:versionId)'}),
mode: flags.string({description: 'Value specifying mode of training (Standard | Neural).'}),
wait: flags.boolean({description: 'Wait until training complete and then display status'}),
json: flags.boolean({description: 'Display output as JSON'}),
}
Expand All @@ -37,7 +38,7 @@ export default class LuisTrainRun extends Command {
utils.validateRequiredProps(requiredProps)

try {
const trainingRequestStatus = await Train.train({subscriptionKey, endpoint, appId}, versionId)
const trainingRequestStatus = await Train.train({subscriptionKey, endpoint, appId}, versionId, flags.mode)
Copy link
Contributor

Choose a reason for hiding this comment

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

Are string flags defaulted to ''?

Copy link
Member Author

Choose a reason for hiding this comment

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

to undefined

Copy link
Contributor

Choose a reason for hiding this comment

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

It's pedantic, but if undefined is a valid type for the mode parameter, the train function signature should be (..., mode?: string). strict mode would raise an error here.

if (trainingRequestStatus) {
await utils.writeToConsole(trainingRequestStatus)
const output = flags.json ? JSON.stringify({Status: 'Success'}, null, 2) : '\nTraining request successfully issued'
Expand All @@ -52,7 +53,7 @@ export default class LuisTrainRun extends Command {
return this.checkTrainingStatus({subscriptionKey, endpoint, appId}, versionId, flags.json)
}
} catch (err) {
throw new CLIError(`Failed to issue training request: ${err}`)
throw new CLIError(`Failed to issue training request: ${err.message}`)
}
}

Expand Down
11 changes: 11 additions & 0 deletions packages/luis/test/commands/luis/train/run.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,4 +53,15 @@ describe('luis:train:run', () => {
expect(ctx.stdout).to.contain('checking training status...')
})

test
.nock('https://westus.api.cognitive.microsoft.com', api => api
.post(uri => uri.includes('train?mode=Standard'))
.reply(202, {"statusId": 2,"status": "UpToDate"})
)
.stdout()
.command(['luis:train:run', '--appId', uuidv1(), '--versionId', '0.1', '--subscriptionKey', uuidv1(), '--endpoint', 'https://westus.api.cognitive.microsoft.com', '--mode', 'Standard'])
.it('issues an asynchronous training request and reports when complete using trainning mode Standard', ctx => {
expect(ctx.stdout).to.contain('Training request successfully issued')
})

})