Fixing the ability to provide custom retryMap #1488

Yahav · 2018-07-30T12:37:15Z

googlebot · 2018-07-30T12:37:19Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.

What to do if you already signed the CLA

Individual signers

It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
The email used to register you as an authorized contributor must also be attached to your GitHub account.

Yahav · 2018-07-30T12:39:10Z

I signed it!

googlebot · 2018-07-30T12:39:13Z

CLAs look good, thanks!

src/Google/Client.php

@@ -795,7 +795,7 @@ public function execute(RequestInterface $request, $expectedClass = null)
    // this is where most of the grunt work is done
    $http = $this->authorize();

-    return Google_Http_REST::execute($http, $request, $expectedClass, $this->config['retry']);
+    return Google_Http_REST::execute($http, $request, $expectedClass, null, $this->config['retry']);


Will be initiated like so: $this->client = new Google_Client(array('retry'=>array('retries'=>99),'retryMap'=>$this->retryMap));

src/Google/Client.php

@@ -795,7 +796,7 @@ public function execute(RequestInterface $request, $expectedClass = null)
    // this is where most of the grunt work is done
    $http = $this->authorize();

-    return Google_Http_REST::execute($http, $request, $expectedClass, $this->config['retry']);
+    return Google_Http_REST::execute($http, $request, $expectedClass, $this->config['retry'], $this->config['retry_map']);


src/Google/Client.php

+        $this->config['retry'],
+        $this->config['retry_map']
+    );
+


Yahav · 2018-08-05T14:10:13Z

Can someone merge this?

dwsupplee · 2018-08-07T21:27:25Z

@mattwhisenhunt is this okay to merge?

ggmartins091 · 2021-03-10T13:47:05Z

Guys, is this right?

I know this fixes the ability to provide the custom retryMap, but why are we passing $this->config['retry'] as the third argument here when in reality it expects the whole config array as can be seen here and here

Even the retry index in that array is incorrect, the Runner class looks for an index called retries. But still, we should pass the whole config array.

Am I missing something here?

dwsupplee · 2021-03-11T17:07:55Z

@ggmartins091 it looks correct to me. $this->config['retry'] is intended to be the configuration array for the Runner, as seen here, not a specific number of retry attempts.

ggmartins091 · 2021-03-11T17:29:18Z

@dwsupplee Oh, my bad. I get it now.

So for me to change the number of retries I would need to do new Google_Client(['retry' => ['retries' => 3]]);

Thats what I was missing. Thank you.

Fixing the ability to provide custom retryMap

df6837f

googleapis#1487

googlebot added the cla: no This human has *not* signed the Contributor License Agreement. label Jul 30, 2018

googlebot added cla: yes This human has signed the Contributor License Agreement. and removed cla: no This human has *not* signed the Contributor License Agreement. labels Jul 30, 2018

mattwhisenhunt requested a review from a team August 1, 2018 16:41

dwsupplee suggested changes Aug 1, 2018

View reviewed changes

Yahav added 3 commits August 1, 2018 22:06

Update Client.php

0193210

Will be initiated like so: $this->client = new Google_Client(array('retry'=>array('retries'=>99),'retryMap'=>$this->retryMap));

forgot a comma

d026ea3

retryMap to retry_map.. conventions

0a2b861

dwsupplee reviewed Aug 1, 2018

View reviewed changes

Yahav added 2 commits August 2, 2018 08:40

line length

21602d3

spacing

40f0716

dwsupplee reviewed Aug 2, 2018

View reviewed changes

src/Google/Client.php Outdated

$this->config['retry'],

$this->config['retry_map']

);

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

removed whitespace

050e723

dwsupplee approved these changes Aug 2, 2018

View reviewed changes

mattwhisenhunt merged commit 9edd15c into googleapis:master Aug 7, 2018

bshaffer mentioned this pull request May 21, 2019

Prepare for v2.2.3 #1633

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixing the ability to provide custom retryMap #1488

Fixing the ability to provide custom retryMap #1488

Yahav commented Jul 30, 2018

googlebot commented Jul 30, 2018

Yahav commented Jul 30, 2018

googlebot commented Jul 30, 2018

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

Yahav commented Aug 5, 2018

dwsupplee commented Aug 7, 2018

ggmartins091 commented Mar 10, 2021 •

edited

Loading

dwsupplee commented Mar 11, 2021

ggmartins091 commented Mar 11, 2021

Fixing the ability to provide custom retryMap #1488

Fixing the ability to provide custom retryMap #1488

Conversation

Yahav commented Jul 30, 2018

googlebot commented Jul 30, 2018

What to do if you already signed the CLA

Individual signers

Corporate signers

Yahav commented Jul 30, 2018

googlebot commented Jul 30, 2018

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

Yahav commented Aug 5, 2018

dwsupplee commented Aug 7, 2018

ggmartins091 commented Mar 10, 2021 • edited Loading

dwsupplee commented Mar 11, 2021

ggmartins091 commented Mar 11, 2021

ggmartins091 commented Mar 10, 2021 •

edited

Loading