Make Speech client use autogen layer #1631

jmdobry · 2016-09-26T23:19:38Z

My initial attempt at using the autogen layer in the handwritten layer. I have a concern:

It would appear that gax-nodejs does not translate camel case option names into the snake case names defined the proto. Where sampleRate worked before, thy tests for recognize and createRecognitionStream started failing until I switched it to sample_rate.

Other than that it seems to work fine.

I found some other gax-nodejs usability issues with pagination that don't apply here.

stephenplusplus · 2016-09-26T23:20:31Z

// @jmuk

Is it possible to update gax-nodejs to expect camelcase args instead of snake? JS convention is generally camelcase. These are the options we had to provide to grpc to enable this: https://github.com/GoogleCloudPlatform/google-cloud-node/blob/95881d2a83904be5f2e4ebc24ee93369db212ac4/packages/common/src/grpc-service.js#L755

packages/speech/src/index.js

  };

  common.GrpcService.call(this, config, options);
+  this.api = new v1beta1(options).speechApi();


packages/speech/src/index.js

  };

  common.GrpcService.call(this, config, options);
+  this.api = {


packages/speech/src/index.js


  common.GrpcService.call(this, config, options);
+  this.api = {
+    speechApi: v1beta1(options).speechApi()


stephenplusplus · 2016-09-26T23:45:32Z

Just adding the don't merge while we wait to see if we can stick to 🐫 💼 .

stephenplusplus · 2016-09-26T23:46:57Z

I found some other gax-nodejs usability issues with pagination that don't apply here.

Anything to call out here, or are you going to file something somewhere else?

jmdobry · 2016-09-26T23:48:21Z

Regarding camel case, it looks like gax-nodejs might need to be updated to output code that passes { convertFieldsToCamelCase: true } here: https://github.com/GoogleCloudPlatform/google-cloud-node/blob/master/packages/speech/src/v1beta1/speech_api.js#L220

Regarding pagination, will file elsewhere.

stephenplusplus · 2016-09-28T01:18:11Z

There's a new release of gax out (0.7.0) which camelCases and upgrades to [email protected]. I believe we're now just waiting on the API files to be generated.

jmdobry · 2016-09-28T02:38:07Z

I'll wait for that PR to be merged then.

stephenplusplus · 2016-09-28T12:07:06Z

The PR with the re-generated files has been merged.

packages/speech/src/index.js

-    };
-
-    self.request(protoOpts, reqOpts, function(err, apiResponse) {
+    self.api.Speech.syncRecognize(config, foundFile, function(err, apiResponse) {


jmdobry · 2016-09-28T16:02:55Z

Updated

coveralls · 2016-09-28T18:23:37Z

Coverage remained the same at 100.0% when pulling 216b575 on jmdobry:gax-speech into 96b845b on GoogleCloudPlatform:master.

[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [jackspeak](https://togithub.com/isaacs/jackspeak) | [`3.4.0` -> `3.4.3`](https://renovatebot.com/diffs/npm/jackspeak/3.4.0/3.4.3) | [![age](https://developer.mend.io/api/mc/badges/age/npm/jackspeak/3.4.3?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/jackspeak/3.4.3?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/jackspeak/3.4.0/3.4.3?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/jackspeak/3.4.0/3.4.3?slim=true)](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes <details> <summary>isaacs/jackspeak (jackspeak)</summary> ### [`v3.4.3`](https://togithub.com/isaacs/jackspeak/compare/v3.4.2...v3.4.3) [Compare Source](https://togithub.com/isaacs/jackspeak/compare/v3.4.2...v3.4.3) ### [`v3.4.2`](https://togithub.com/isaacs/jackspeak/compare/v3.4.1...v3.4.2) [Compare Source](https://togithub.com/isaacs/jackspeak/compare/v3.4.1...v3.4.2) ### [`v3.4.1`](https://togithub.com/isaacs/jackspeak/compare/v3.4.0...v3.4.1) [Compare Source](https://togithub.com/isaacs/jackspeak/compare/v3.4.0...v3.4.1) </details> --- ### Configuration 📅 **Schedule**: Branch creation - "after 9am and before 3pm" (UTC), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/googleapis/gax-nodejs).

googlebot added the cla: yes This human has signed the Contributor License Agreement. label Sep 26, 2016

stephenplusplus added the api: speech Issues related to the Speech-to-Text API. label Sep 26, 2016

stephenplusplus reviewed Sep 26, 2016

View reviewed changes

packages/speech/src/index.js Outdated

};

common.GrpcService.call(this, config, options);

this.api = new v1beta1(options).speechApi();

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

jmdobry commented Sep 26, 2016

View reviewed changes

packages/speech/src/index.js

};

common.GrpcService.call(this, config, options);

this.api = {

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

jmdobry commented Sep 26, 2016

View reviewed changes

packages/speech/src/index.js Outdated

common.GrpcService.call(this, config, options);

this.api = {

speechApi: v1beta1(options).speechApi()

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

stephenplusplus added the don't merge label Sep 26, 2016

jmdobry mentioned this pull request Sep 27, 2016

Need to default convertFieldsToCamelCase to true googleapis/gax-nodejs#43

Closed

evancohen mentioned this pull request Sep 28, 2016

speech: fix docs to use correct event #1639

Closed

stephenplusplus reviewed Sep 28, 2016

View reviewed changes

packages/speech/src/index.js Outdated

};

self.request(protoOpts, reqOpts, function(err, apiResponse) {

self.api.Speech.syncRecognize(config, foundFile, function(err, apiResponse) {

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

jmdobry and others added 2 commits September 28, 2016 09:02

Use gax-nodejs for syncRecognize and asyncRecognize.

8895600

Merge 8895600 into 96b845b

46d1c98

fix tests

216b575

googlebot added cla: no This human has *not* signed the Contributor License Agreement. and removed cla: yes This human has signed the Contributor License Agreement. labels Sep 28, 2016

stephenplusplus added cla: yes This human has signed the Contributor License Agreement. and removed don't merge cla: no This human has *not* signed the Contributor License Agreement. labels Sep 28, 2016

stephenplusplus merged commit 0f8d832 into googleapis:master Sep 28, 2016

jmdobry deleted the gax-speech branch September 28, 2016 17:36

sofisl pushed a commit that referenced this pull request Jan 24, 2023

Make Speech client use autogen layer (#1631)

01e430a

sofisl pushed a commit that referenced this pull request Jan 25, 2023

Make Speech client use autogen layer (#1631)

1882d4a

Make Speech client use autogen layer #1631

Make Speech client use autogen layer #1631

Uh oh!

Conversation

jmdobry commented Sep 26, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

stephenplusplus commented Sep 26, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

stephenplusplus commented Sep 26, 2016

Uh oh!

stephenplusplus commented Sep 26, 2016

Uh oh!

jmdobry commented Sep 26, 2016

Uh oh!

stephenplusplus commented Sep 28, 2016

Uh oh!

jmdobry commented Sep 28, 2016

Uh oh!

stephenplusplus commented Sep 28, 2016

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

jmdobry commented Sep 28, 2016

Uh oh!

coveralls commented Sep 28, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

jmdobry commented Sep 26, 2016 •

edited

Loading

stephenplusplus commented Sep 26, 2016 •

edited

Loading