Skip to content
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

fix(speech): increment retry setting values and test timeout values in IT #9188

Merged
merged 9 commits into from
Mar 4, 2023

Conversation

diegomarquezp
Copy link
Contributor

@diegomarquezp diegomarquezp commented Mar 3, 2023

@product-auto-label product-auto-label bot added the api: speech Issues related to the Speech-to-Text API. label Mar 3, 2023
@diegomarquezp diegomarquezp requested a review from a team March 3, 2023 18:46
@burkedavison
Copy link
Contributor

Is this in response to an issue with speech IT flakiness? The nightlies haven't been showing an issue here - but is it showing up elsewhere?

Copy link
Member

@meltsufin meltsufin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please reference an issue in the description, if there is one for this flake.

@@ -14,7 +14,6 @@ Java idiomatic client for [Cloud Speech][product-docs].

If you are using Maven, add this to your pom.xml file:

<!--- {x-version-update-start:google-cloud-speech:released} -->
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ddixit14 Why is OwlBot removing this?

@diegomarquezp
Copy link
Contributor Author

@meltsufin @burkedavison I added context in the description

@meltsufin
Copy link
Member

@diegomarquezp This seems similar to an issue that was reported by a customer in the past: https://github.com/googleapis/java-speech/issues/207.
Please make sure you're applying the suggestion there that seems to have worked.

.setInitialRpcTimeout(Duration.ZERO)
.setRpcTimeoutMultiplier(1.0)
.setMaxRpcTimeout(Duration.ZERO)
.setTotalTimeout(Duration.ofDays(1))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like a pretty long timeout.

Copy link
Contributor

@lqiu96 lqiu96 Mar 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe the default timeout is set to be 5 min. I think you can probably just double it to 10 minutes and should be fine. There was an issue from the java-doc-samples repo a few weeks ago about a similar LRO issue and the runtime increased from ~4:30min to ~5:30min.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using a more reasonable ofHours(1)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not see your suggestion @lqiu96 . Let's try 10 minutes

@diegomarquezp diegomarquezp merged commit b72fe9d into main Mar 4, 2023
@diegomarquezp diegomarquezp deleted the speech-it-retry-fix branch March 4, 2023 03:34
@release-please release-please bot mentioned this pull request Mar 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: speech Issues related to the Speech-to-Text API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants