FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Fix Speech LRO handling and add HTTP side for multiple results. by daspecster · Pull Request #2965 · googleapis/google-cloud-python · GitHub

/ google-cloud-python Public

Fix Speech LRO handling and add HTTP side for multiple results. - #2965

Merged
daspecster merged 3 commits into
googleapis:masterfrom
daspecster:fix-speech-system-tests
Feb 6, 2017
ConversationCommits (3)ChecksFiles changed
Merged

Fix Speech LRO handling and add HTTP side for multiple results. #2965
daspecster merged 3 commits into
googleapis:masterfrom
daspecster:fix-speech-system-tests

Conversation

daspecster commented Jan 26, 2017 •
edited
Loading

Copy link
Copy Markdown
Contributor

This is an update to fill out the HTTP side of #2962 and I have a hacky but effective way to handle the new LRO.

I tried using the OperationsClient but for some reason it just kept 404ing. I'm not sure what I was doing wrong.

/cc @lukesneeringer

daspecster added the api: speech Issues related to the Speech-to-Text API. label Jan 26, 2017
googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jan 26, 2017
daspecster commented Jan 26, 2017
View reviewed changes
Comment thread speech/google/cloud/speech/_gax.py Outdated
operation_future = api.async_recognize(config=config, audio=audio)

return Operation.from_pb(response, self)
return Operation.from_pb(operation_future._operation, self)

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

This comment was marked as spam.

Sign in to view

lukesneeringer commented Jan 26, 2017 •
edited
Loading

Copy link
Copy Markdown
Contributor

Ideally, I would prefer the user get the OperationFuture object from GAX, and would like to look into why @daspecster is having trouble making it work. I will try to do that in the next day or so.

That said, merging this is almost certainly preferable to (1) reverting #2962 (which fixes an item the Speech team identified as critical) or (2) leaving async broken. :-)

daspecster mentioned this pull request Jan 27, 2017
Merged

daspecster commented Jan 27, 2017

Copy link
Copy Markdown
Contributor Author

@eoogbe I was getting a Rendezvous 404 when I tried to use OperationsClient.get_operation().
I pretty much followed the example, but I had to add the scope.

In this PR I'm just pulling _OperationFuture._operation but I'm sure that's not the intended way it should be used.

Any help would be greatly appreciated!

daspecster commented Jan 27, 2017

Copy link
Copy Markdown
Contributor Author

@lukesneeringer the _OperationFuture usage is updated. PTAL

lukesneeringer suggested changes Jan 30, 2017
View reviewed changes
Comment thread speech/google/cloud/speech/_gax.py
operation_future = api.async_recognize(config=config, audio=audio)

return Operation.from_pb(response, self)
return Operation.from_pb(operation_future.last_operation_data(), self)

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

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

daspecster mentioned this pull request Feb 1, 2017
Merged

daspecster commented Feb 2, 2017

Copy link
Copy Markdown
Contributor Author

@lukesneeringer do you have anything else for this?

lukesneeringer commented Feb 2, 2017

Copy link
Copy Markdown
Contributor

Yeah, I am writing an _OperationFuture clone to go into google.cloud.core this morning.

lukesneeringer commented Feb 3, 2017

Copy link
Copy Markdown
Contributor

Okay, I am aborting that for now. The rabbit hole is going too far. Yes, you can merge this.

daspecster merged commit de65409 into googleapis:master Feb 6, 2017
daspecster deleted the fix-speech-system-tests branch February 6, 2017 15:38
daspecster mentioned this pull request Feb 24, 2017
Merged
19 tasks
richkadel pushed a commit to richkadel/google-cloud-python that referenced this pull request May 6, 2017
Fix Speech LRO handling and add HTTP side for multiple results. (goog…
fee39bf 
…leapis#2965)

* Update after googleapis#2962 to fill out http side and handle new LRO.

* Update _OperationsFuture usage.

* Mock OperationsClient.
atulep pushed a commit that referenced this pull request Apr 3, 2023
Fix Speech LRO handling and add HTTP side for multiple results. (#2965)
0d972c7 
* Update after #2962 to fill out http side and handle new LRO.

* Update _OperationsFuture usage.

* Mock OperationsClient.
atulep pushed a commit that referenced this pull request Apr 18, 2023
Fix Speech LRO handling and add HTTP side for multiple results. (#2965)
423aebf 
* Update after #2962 to fill out http side and handle new LRO.

* Update _OperationsFuture usage.

* Mock OperationsClient.
parthea pushed a commit that referenced this pull request Oct 22, 2023
Fix Speech LRO handling and add HTTP side for multiple results. (#2965)
fca6333 
* Update after #2962 to fill out http side and handle new LRO.

* Update _OperationsFuture usage.

* Mock OperationsClient.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

3 more reviewers
bjwatson bjwatson left review comments
evaogbe evaogbe left review comments
lukesneeringer lukesneeringer approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

lukesneeringer

tseaver

Labels

api: speech Issues related to the Speech-to-Text API. cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

Back | FazBrowse Home | New Git URL