-
Notifications
You must be signed in to change notification settings - Fork 6
Mt sdk #69
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
base: master
Are you sure you want to change the base?
Mt sdk #69
Changes from all commits
f2bca68
b97cd26
9b8d4a2
c9addc1
5982b9b
37740dc
aa0fb5c
da60b6a
d51c7ed
2a5ea04
7731bef
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -29,15 +29,19 @@ let Handler = function () { | |
|
|
||
| /** | ||
| * Initializes the handler, sets up the connection to wcs | ||
| * @params credentials which includes apikey, workspace_id, workspace_name, iam_apikey description, iam_apikey, workspace url, version, version_date | ||
| */ | ||
| Handler.prototype.initialize = function () { | ||
| if(WaEnvUtils.isValidWaEnvVars()) { | ||
| Handler.prototype.initialize = function (credentials) { | ||
| //check if credentials are coming from the handler initialize call and also valid from the index.js of the skill | ||
| if((credentials && WaEnvUtils.isValidWaCredentials(credentials))){ | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is a specific check for a specific type of NLU engine here. How can you cover here all future types? |
||
| this.wcsCredentials = WaEnvUtils.setupWCSCredentialsfromVCAP(credentials); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this is only way we can pass credentials? from VCAP SERVICES? |
||
| } else if(WaEnvUtils.isValidWaEnvVars()) { | ||
| this.wcsCredentials = WaEnvUtils.setupWCSCredentialsFromEnv(); | ||
| } else { | ||
| let rawJson = fs.readFileSync(path.join(process.env.skillSDKResDir, nluFolder, wcsFileName)); | ||
| this.wcsCredentials = JSON.parse(rawJson); | ||
| } | ||
| if (this.wcsCredentials) { | ||
| if(this.wcsCredentials){ | ||
| let credentials = this.wcsCredentials.credentials; | ||
| setupWcs(this, credentials.url, credentials.username, credentials.password, credentials.version_date, credentials.version, credentials.iam_apikey) | ||
| } | ||
|
|
@@ -339,7 +343,8 @@ let setupWcs = function (handler, wcsUrl, wcsUsername, wcsPassword, versionDate, | |
| options = { | ||
| version: version, | ||
| iam_apikey: apiKey, | ||
| url: wcsUrl | ||
| url: wcsUrl, | ||
| export: false | ||
| } | ||
|
|
||
| } else { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -96,13 +96,13 @@ class Factory { | |
| }); | ||
| } | ||
|
|
||
|
|
||
| static getNLUs(manifest) { | ||
| //pass the credentials sent from the skill to getNLU's | ||
| static getNLUs(manifest, credentials) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How one set of credentials can cover ALL types of engines? |
||
| return new Promise(function (resolve, reject) { | ||
| let promises = []; | ||
| let types = manifest.nlu; | ||
| types.forEach(function (type) { | ||
| promises.push(getNLU(type, manifest.name)); | ||
| promises.push(getNLU(type, manifest.name, credentials)); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if the skill uses also a regexp engine what credentials will be passed? it doesn't use any credentials
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. From the skill, we will not be passing the credentials if the nlu is of the type regexp.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But what if you have regexp AND WCS? |
||
| }); | ||
|
|
||
| Promise.all(promises.map(reflect)).then(function (results) { | ||
|
|
@@ -124,11 +124,14 @@ class Factory { | |
|
|
||
| module.exports = Factory; | ||
|
|
||
|
|
||
| function getNLU(type, name) { | ||
| //pass the credentials from the skill | ||
| function getNLU(type, name, credentials) { | ||
| return new Promise(function (resolve, reject) { | ||
| let nlu; | ||
| if(WaEnvUtils.isValidWaEnvVars() && type === 'wcs') { | ||
| if(credentials && WaEnvUtils.isValidWaCredentials(credentials) && type === 'wcs'){ | ||
| nlu = WaEnvUtils.setupWCSCredentialsfromVCAP(credentials); | ||
| } | ||
| else if(WaEnvUtils.isValidWaEnvVars() && type === 'wcs') { | ||
| nlu = WaEnvUtils.setupWCSCredentialsFromEnv(); | ||
| } else { | ||
| nlu = require(`${resPath}/nlu/${type}`); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,6 +19,17 @@ let isValidWaEnvVars = function() { | |
| process.env.WCS_WORKSPACE_LANGUAGE)); | ||
| }; | ||
|
|
||
| /** | ||
| * check that all the WA credentials sent from the skill are valid | ||
| * @returns {string} | ||
| */ | ||
|
|
||
| let isValidWaCredentials = function(credentials) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You only check here WCS? what about other NLUs? |
||
| return ((process.env.WCS_USERNAME && process.env.WCS_URL && process.env.WCS_PASSWORD && process.env.WCS_VERSION_DATE | ||
| && process.env.WCS_VERSION) || | ||
| (credentials.apikey && process.env.WCS_WORKSPACE_LANGUAGE && credentials.workspace_name && credentials.workspace_name && credentials.url)); | ||
| }; | ||
|
|
||
|
|
||
| /** | ||
| * Sets up the handler's wcs credentials from environment variables | ||
|
|
@@ -55,8 +66,47 @@ let setupWCSCredentialsFromEnv = function () { | |
| return options; | ||
| }; | ||
|
|
||
| /** | ||
| * Sets up the handler's wcs credentials from VCAP credentials read from the skill | ||
| * @param handler | ||
| */ | ||
|
|
||
| let setupWCSCredentialsfromVCAP = function (credentials) { | ||
|
|
||
| let options = { | ||
| "workspace": { | ||
| [process.env.WCS_WORKSPACE_LANGUAGE]: { | ||
| "name": credentials.workspace_name, | ||
| "workspace_id": credentials.workspace_id | ||
| } | ||
| } | ||
| }; | ||
| // new way of wa authentication - with iam api-key | ||
| if(credentials.apikey) { | ||
| options.credentials = { | ||
| "url": credentials.url, | ||
| "version": credentials.version_date, | ||
| "iam_apikey": credentials.apikey | ||
| } | ||
| } | ||
| // deprecated way of wa authentication | ||
| else if(process.env.WCS_PASSWORD && process.env.WCS_USERNAME) { | ||
| options.credentials = { | ||
| "url": process.env.WCS_URL, | ||
| "version": process.env.WCS_VERSION, | ||
| "version_date": process.env.WCS_VERSION_DATE, | ||
| "password": process.env.WCS_PASSWORD, | ||
| "username": process.env.WCS_USERNAME | ||
| } | ||
| } | ||
| return options; | ||
|
|
||
| }; | ||
|
|
||
|
|
||
| module.exports = { | ||
| isValidWaEnvVars: isValidWaEnvVars, | ||
| setupWCSCredentialsFromEnv: setupWCSCredentialsFromEnv | ||
| isValidWaCredentials: isValidWaCredentials, | ||
| setupWCSCredentialsFromEnv: setupWCSCredentialsFromEnv, | ||
| setupWCSCredentialsfromVCAP: setupWCSCredentialsfromVCAP | ||
| }; | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
where is this called from? traditionally it was called by the skill on startup, if this has not changed then how does it get the credentials? if it has changed then are you creating a new handler for each request?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@offerakrabi it is still being called from the skill only. The skill is getting credentials on start up from VCAP_SERVICES of the bound conversation service and then makes a WA API call to get the workspace_id based on the skill name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ashokbabuy This is a singleton - meaning this is the ONLY instance for ALL calls, including all NLU engines.
It is wrong to pass credentials if it is not related to ALL engines.
What do you in a case where there is an engine that is not WCS? that requires different credentials IN ADDITION to WCS?
And how is it multi tenant if you pass here only 1 set of credentials?