Skip to content

core: correctly locate maxRetries option - #1358

Merged
callmehiphop merged 1 commit into
googleapis:masterfrom
stephenplusplus:spp--1357
Jun 6, 2016
Merged

callmehiphop merged 1 commit into
googleapis:masterfrom
stephenplusplus:spp--1357

Conversation

@stephenplusplus

Copy link
Copy Markdown
Contributor

Fixes #1357

We were trying to find the user's maxRetries option on the config argument, instead of options.

Additionally, retry-request expects an argument named retries, where we were calling it maxRetries.

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jun 6, 2016
@coveralls

coveralls commented Jun 6, 2016 •

Copy link
Copy Markdown

Coverage Status

Changes Unknown when pulling 06b5e17 on stephenplusplus:spp--1357 into * on GoogleCloudPlatform:master*.

@stephenplusplus

Copy link
Copy Markdown
Contributor Author

@coveralls what can we do to make you happy?

@callmehiphop

Copy link
Copy Markdown
Contributor

I know we decided to not add system tests for every service in regards to this, but maybe one grpc system test test would benefit us here. WDYT?

@stephenplusplus

Copy link
Copy Markdown
Contributor Author

What would the system test look like?

@callmehiphop

Copy link
Copy Markdown
Contributor

I was going to suggest mitm but I now remember that doesn't work with grpc requests. Is this basically untestable (outside of unit tests)?

@stephenplusplus

stephenplusplus commented Jun 6, 2016 •

Copy link
Copy Markdown
Contributor Author

Yeah, we are just going on trust that retry-request works. Which sounds worse than it is. That's essentially what we do with all of our dependencies... we scope them out, see if the code looks good, that it's tested... then if we trust it, we let its own test suite worry about covering features that it says it offers.

But yeah, testing that we do retry logic when we get a retry-able response from gRPC is quite untestable. I don't think we can control when gRPC will give us, so it's just meant to stay in unit testing-land.

@callmehiphop
callmehiphop merged commit e4c3d55 into googleapis:master Jun 6, 2016
sofisl pushed a commit that referenced this pull request Feb 3, 2026
This reverts commit 139242258dd614fcc226ad8f6db58618d9e8b453.
sofisl pushed a commit that referenced this pull request Feb 25, 2026
* fix: capitalize action in Bucket#addLifecycleRule

* chore: increase unit test coverage for Bucket#addLifecycleRule
sofisl pushed a commit that referenced this pull request Feb 26, 2026
The emulator now supports the `NUMERIC` data type.
thiyaguk09 pushed a commit to thiyaguk09/google-cloud-node-fork that referenced this pull request Mar 18, 2026
* fix: capitalize action in Bucket#addLifecycleRule

* chore: increase unit test coverage for Bucket#addLifecycleRule
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes This human has signed the Contributor License Agreement. core

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants