Skip to content

BIG-168 Add retries for GoogleClient #73

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

Merged
merged 4 commits into from
Oct 10, 2023
Merged

Conversation

jirkasemmler
Copy link
Contributor

@jirkasemmler jirkasemmler commented Oct 9, 2023

Jira: https://keboola.atlassian.net/browse/BIG-168

  • vetsina klientu krome Google_client ma nejakou retry policy nastavenou. Kdyz se do nich prokliknes, tak je tam neco jako 'clientConfig' => __DIR__ . '/../resources/cloud_billing_client_config.json',, kde ten json obsahuje nejakou retry policy na ruzne metody. Do toho jsem nechtel uplne stourat
  • takze jsem ten retry pridal do Google_client, ktery ho nema a resi nam to problem zakladani SA
  • pridal jsem i takovej zvlastni test. Je diskutabilni, zda ho tam vubec nechat, ale ty si muzes vyzkouset, jak to frci. kdyz si jeste do Google\Task\Runner pridas na lajnu 187 neco takoveho echo time() . " executing #{$this->attempts} / {$this->maxAttempts} \n"; tak uvidis jak to pekne dela retry
  • nastaveni DEFAULT_RETRY_SETTINGS mam tak nejak od oka. Vime, ze potrebujeme cekat cca 60s, takze delay je 60s, factor jsem dal jen 1.1 aby to moc exponencialne nerostlo a jitter taky malinkatej

@jirkasemmler jirkasemmler requested review from a team and zajca and removed request for a team October 9, 2023 23:43
Copy link
Member

@zajca zajca left a comment

Choose a reason for hiding this comment

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

fakčí to, ten test bych nemazal, ale skipnul bych ho.

// try max 3 times
'retries' => 3,
// multiplicator of backoff time between runs. First = $initial_delay ; second $previousDelay * $factor
'factor' => 1.1,
Copy link
Member

Choose a reason for hiding this comment

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

hmm mohlo to byt lineární, ale je tam ten jitter, který to može posunůt před tů minutu takže asi cajk.

$this->cleanTestProject();
}

public function testCreateManyServiceACounts(): void
Copy link
Member

Choose a reason for hiding this comment

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

asi bych to dal mark as skip, je to zbytečné imho to pokaždé půšťat

@jirkasemmler jirkasemmler requested a review from zajca October 10, 2023 11:19
@zajca
Copy link
Member

zajca commented Oct 10, 2023

upravil sem tu test metodu, nemá to žádné dobré řešení phpstan/phpstan-phpunit#52

@jirkasemmler jirkasemmler merged commit 4927db2 into main Oct 10, 2023
@jirkasemmler jirkasemmler deleted the jirka-big-168-retry branch October 10, 2023 21:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants