Skip to content

Commit 7ee035c

Browse files
committed
bug #94 Add a custom retry strategy to retry 404 (Nyholm)
This PR was squashed before being merged into the master branch. Discussion ---------- Add a custom retry strategy to retry 404 ### Problem Sometimes, we get a webhook request and then we trying to ask the Github api for that issue, but the API says 404. ### Acknowledgement I know I do more service definitions than needed, but I didnt want to pull in FrameworkBundle:5.2 ### Excuse The changes in src/Issues/GitHub/GitHubStatusApi.php is just CS.. I started to read/think there before I ended up with the RetryStrategy solution ### Shout out @jderusse, this is really awesome =) Commits ------- c04b622 Rebase fix a4998fc Add a custom retry strategy to retry 404
2 parents 08b8ff5 + c04b622 commit 7ee035c

File tree

4 files changed

+43
-75
lines changed

4 files changed

+43
-75
lines changed

composer.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
"symfony/dotenv": "^5.1",
1313
"symfony/flex": "^1.8",
1414
"symfony/framework-bundle": "^5.1",
15-
"symfony/http-client": "^5.1",
15+
"symfony/http-client": "^v5.2.0-BETA3",
1616
"symfony/monolog-bundle": "~3.5",
1717
"symfony/security-core": "5.1.*",
1818
"symfony/twig-pack": "^1.0",
@@ -38,7 +38,7 @@
3838
"extra": {
3939
"symfony": {
4040
"allow-contrib": true,
41-
"require": "5.1.*"
41+
"require": "^5.1"
4242
}
4343
},
4444
"autoload": {

composer.lock

Lines changed: 13 additions & 47 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

config/packages/github_api.yaml

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,26 @@ services:
77

88
Github\HttpClient\Builder:
99
arguments:
10-
- '@?Http\Client\HttpClient'
11-
- '@?Http\Message\RequestFactory'
12-
- '@?Http\Message\StreamFactory'
10+
- '@github.httplug_client'
11+
- '@Http\Message\RequestFactory'
12+
- '@Http\Message\StreamFactory'
13+
14+
github.httplug_client:
15+
class: Symfony\Component\HttpClient\HttplugClient
16+
arguments:
17+
- '@github.retryable_client'
18+
- '@Psr\Http\Message\ResponseFactoryInterface'
19+
- '@Psr\Http\Message\StreamFactoryInterface'
20+
21+
github.retryable_client:
22+
class: Symfony\Component\HttpClient\RetryableHttpClient
23+
arguments:
24+
- '@http_client'
25+
- '@github.retry_strategy'
26+
- 2
27+
- '@logger'
28+
29+
github.retry_strategy:
30+
class: Symfony\Component\HttpClient\Retry\GenericRetryStrategy
31+
arguments:
32+
- [0, 404, 423, 425, 429, 500, 502, 503, 504, 507, 510]

src/Issues/GitHub/GitHubStatusApi.php

Lines changed: 5 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -45,20 +45,12 @@ public function setIssueStatus($issueNumber, $newStatus, Repository $repository)
4545
}
4646

4747
$newLabel = null === $newStatus ? null : self::$statusToLabel[$newStatus];
48-
$this->logger->info(sprintf(
49-
'Fetching issue labels for issue %s, repository %s',
50-
$issueNumber,
51-
$repository->getFullName()
52-
));
48+
$this->logger->info(sprintf('Fetching issue labels for issue %s, repository %s', $issueNumber, $repository->getFullName()));
5349
$currentLabels = $this->labelsApi->getIssueLabels($issueNumber, $repository);
5450

55-
$this->logger->info(sprintf(
56-
'Fetched the following labels: %s',
57-
implode(', ', $currentLabels)
58-
));
51+
$this->logger->info(sprintf('Fetched the following labels: %s', implode(', ', $currentLabels)));
5952

6053
$addLabel = true;
61-
6254
foreach ($currentLabels as $label) {
6355
// Ignore non-status, except when the bug is reviewed
6456
// but still marked as unconfirmed.
@@ -75,23 +67,13 @@ public function setIssueStatus($issueNumber, $newStatus, Repository $repository)
7567
}
7668

7769
// Remove other statuses
78-
$this->logger->debug(sprintf(
79-
'Removing label %s from issue %s on repository %s',
80-
$label,
81-
$issueNumber,
82-
$repository->getFullName()
83-
));
70+
$this->logger->debug(sprintf('Removing label %s from issue %s on repository %s', $label, $issueNumber, $repository->getFullName()));
8471
$this->labelsApi->removeIssueLabel($issueNumber, $label, $repository);
8572
}
8673

8774
// Ignored if the label is already set
8875
if ($addLabel && $newLabel) {
89-
$this->logger->debug(sprintf(
90-
'Adding label "%s" to issue %s on repository %s',
91-
$newLabel,
92-
$issueNumber,
93-
$repository->getFullName()
94-
));
76+
$this->logger->debug(sprintf('Adding label "%s" to issue %s on repository %s', $newLabel, $issueNumber, $repository->getFullName()));
9577
$this->labelsApi->addIssueLabel($issueNumber, $newLabel, $repository);
9678
$this->logger->debug('Label added!');
9779
}
@@ -108,7 +90,7 @@ public function getIssueStatus($issueNumber, Repository $repository)
10890
}
10991

11092
// No status set
111-
return;
93+
return null;
11294
}
11395

11496
public static function getNeedsReviewLabel()

0 commit comments

Comments
 (0)