-
Notifications
You must be signed in to change notification settings - Fork 9
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
Feature/watson transcribe #356
Conversation
selfie |
@@ -0,0 +1,106 @@ | |||
// This endpoint handles server calls to get transcript of an audio file |
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.
@keving17 This is really well done with the way you've factored this into the different pieces! 👍
|
||
pool.query(sql,values) | ||
.then(results => { | ||
if ((results.rowCount >= 1) && (results.rows[0].transcript !== "")){ |
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.
Would adding LIMIT 1
to the SQL simplify these checks?
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.
So I think that since I use the 0th item in rows anyways, it is probably fine to have LIMIT 1
.
Do you think it would be better to use LIMIT 1
and just have the if statement be
if ((results.rowCount === 1) && (results.rows[0].transcript !== "")){
I'm not sure if I'm missing the point.
module.exports = function createWatsonClient() { | ||
const watsonConfig = (process.env.NODE_ENV === 'development') | ||
? JSON.parse(fs.readFileSync('./tmp/watson_config.json')) | ||
: JSON.parse(process.env.WATSON_CONFIG_JSON); |
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.
This probably needs a line in the README.
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.
yep! I added this to issue #350
return result.json(); | ||
}) | ||
.catch(err => { | ||
console.log('Failed to request transcript'); |
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.
should this return a failure status for the HTTP 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.
hmmmm. So I think I generally have the return statuses be set on the server side. In this case, in getAudio.js, the insecureStreamAudioFileFromS3 returns a 200 by default if it can find the audio file and returns a 503 if it can't get an audio file from s3.
Using IBM Watson Speech-To-Text to transcribe audio files to text for analyses tools.
I started this work over in #349 but cut a new branch from a newer part of feature/authentication to avoid having to catch up the old feature/transcription branch.
Will need to insert watson credentials into production environment for this to work