Upgrades vision to partial-GAPIC #434
Conversation
vision/detect.js
Outdated
| @@ -62,18 +59,20 @@ function detectFacesGCS (bucketName, fileName) { | |||
| // The path to the file within the bucket, e.g. "path/to/image.png" | |||
| // const fileName = 'path/to/image.png'; | |||
|
|
|||
| // Performs face detection on the remote file | |||
| vision.detectFaces(storage.bucket(bucketName).file(fileName)) | |||
| const gcsPath = `gs://` + bucketName + `/` + fileName; | |||
There was a problem hiding this comment.
change to
const gcsPath = `gs://${bucketName}/${fileName}`;
vision/detect.js
Outdated
| vision.detectLabels(storage.bucket(bucketName).file(fileName)) | ||
| .then((results) => { | ||
| const labels = results[0]; | ||
| const gcsPath = `gs://` + bucketName + `/` + fileName; |
There was a problem hiding this comment.
change to
const gcsPath = `gs://${bucketName}/${fileName}`;
vision/detect.js
Outdated
| vision.detectLandmarks(storage.bucket(bucketName).file(fileName)) | ||
| .then((results) => { | ||
| const landmarks = results[0]; | ||
| const gcsPath = `gs://` + bucketName + `/` + fileName; |
There was a problem hiding this comment.
change to
const gcsPath = `gs://${bucketName}/${fileName}`;
vision/detect.js
Outdated
| vision.detectText(storage.bucket(bucketName).file(fileName)) | ||
| .then((results) => { | ||
| const detections = results[0]; | ||
| const gcsPath = `gs://` + bucketName + `/` + fileName; |
There was a problem hiding this comment.
change to
const gcsPath = `gs://${bucketName}/${fileName}`;
vision/detect.js
Outdated
| vision.detectLogos(storage.bucket(bucketName).file(fileName)) | ||
| .then((results) => { | ||
| const logos = results[0]; | ||
| const gcsPath = `gs://` + bucketName + `/` + fileName; |
There was a problem hiding this comment.
change to
const gcsPath = `gs://${bucketName}/${fileName}`;
vision/detect.js
Outdated
| @@ -621,10 +605,12 @@ function detectFulltextGCS (bucketName, fileName) { | |||
| // The path to the file within the bucket, e.g. "path/to/image.png" | |||
| // const fileName = 'my-file.jpg'; | |||
|
|
|||
| const gcsPath = `gs://` + bucketName + `/` + fileName; | |||
There was a problem hiding this comment.
change to
const gcsPath = `gs://${bucketName}/${fileName}`;| callback(null, faces); | ||
| }) | ||
| .catch((err) => { | ||
| console.error('ERROR:', err); |
There was a problem hiding this comment.
Please add callback(err); below console.error
| @@ -64,12 +67,18 @@ function highlightFaces (inputFile, faces, outputFile, Canvas, callback) { | |||
| context.strokeStyle = 'rgba(0,255,0,0.8)'; | |||
| context.lineWidth = '5'; | |||
|
|
|||
| faces.forEach(function (face) { | |||
| faces.forEach((face) => { | |||
There was a problem hiding this comment.
Don't use an arrow function here if you're not going to convert the whole file to arrow functions.
There was a problem hiding this comment.
FYI: I converted the whole file to arrow functions. 😛
| }); | ||
| } | ||
| }; | ||
| const cmd = `node quickstart.js`; |
There was a problem hiding this comment.
I don't see how the changes to this file could work. vision/quickstart.js has a hard-coded projectId (it's YOUR_PROJECT_ID). This test will result in failure. The hard-coded projectId in the quickstarts is the reason the quickstarts' tests mock the client library and intercept the call to replace YOUR_PROJECT_ID with a real project ID.
There was a problem hiding this comment.
It works because the API allows 10ish API calls before requiring the caller be authenticated. I'm pretty confident we can remove the code passing the project ID when the client library is initialized but I had assumed there was a reason we were passing the project ID explicitly (e.g. some EAP customers may need to do this).
There was a problem hiding this comment.
We pass it explicitly because that's how all of our "quickstart" samples were designed. For the quickstarts we chose to be explicit about what project is being used.
vision/textDetection.js
Outdated
| }); | ||
| vision.batchAnnotateImages({requests: requests}) | ||
| .then((results) => { | ||
| // console.log(JSON.stringify(results, undefined, 2)); |
There was a problem hiding this comment.
vision/system-test/quickstart.test.js needs to be updated to use labelDetection instead of detectLabels: https://github.com/GoogleCloudPlatform/nodejs-docs-samples/blob/master/vision/system-test/quickstart.test.js
gguuss
force-pushed
the
vision-semi-gapic
branch
from
ba60ea2
to
c00ba7a
Compare
|
So there's good news and bad news. 👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there. 😕 The bad news is that it appears that one or more commits were authored by someone other than the pull request submitter. We need to confirm that they're okay with their commits being contributed to this project. Please have them confirm that here in the pull request. Note to project maintainer: This is a terminal state, meaning the |
Codecov Report
@@ Coverage Diff @@
## master #434 +/- ##
===========================================
+ Coverage 13.55% 83.84% +70.28%
===========================================
Files 3 4 +1
Lines 413 421 +8
===========================================
+ Hits 56 353 +297
+ Misses 357 68 -289
Continue to review full report at Codecov.
|
|
I signed it! |
No description provided.