Skip to content

Commit da33cb6

Browse files
committed
patchwork: derive check context field from CI job name
Currently for BPF CI the "context" field of a checks is effectively a constant with enumerator, for example: bpf/vmtest-bpf-next-VM_Test-48 This has an undesirable side-effect: different checks (such as "AI review" vs "test_progs") may overwrite each other, depending on when they run: for every PR update or for every new revision of the series. Change how the context identifier is constructed: slugify the job name, so that the check is associated with a job, independent of the order or run time. Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev>
1 parent 9d28ade commit da33cb6

File tree

3 files changed

+30
-20
lines changed

3 files changed

+30
-20
lines changed

kernel_patches_daemon/branch_worker.py

Lines changed: 11 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,12 @@
5656
)
5757
from kernel_patches_daemon.github_connector import GithubConnector
5858
from kernel_patches_daemon.github_logs import GithubLogExtractor
59-
from kernel_patches_daemon.patchwork import Patchwork, Series, Subject
59+
from kernel_patches_daemon.patchwork import (
60+
Patchwork,
61+
Series,
62+
slugify_check_context,
63+
Subject,
64+
)
6065
from kernel_patches_daemon.stats import HistogramMetricTimer
6166
from kernel_patches_daemon.status import (
6267
gh_conclusion_to_status,
@@ -100,9 +105,7 @@
100105
"Commenting is disabled on issues with more than 2500 comments"
101106
}
102107
CI_APP = 15368 # GithubApp(url="/apps/github-actions", id=15368)
103-
CI_VMTEST_NAME = "VM_Test"
104108

105-
CI_DESCRIPTION = "vmtest"
106109
MERGE_CONFLICT_LABEL = "merge-conflict"
107110
UPSTREAM_REMOTE_NAME = "upstream"
108111

@@ -650,15 +653,6 @@ def _is_outdated_pr(pr: PullRequest) -> bool:
650653

651654

652655
class BranchWorker(GithubConnector):
653-
@staticmethod
654-
def slugify_context(s: str):
655-
# According to patchwork rule:
656-
# https://github.com/getpatchwork/patchwork/blob/2aa4742ec88be4cd07f569805d22a35c08a08f40/releasenotes/notes/slugify-check-context-dc586f204b5058a7.yaml
657-
# the context need to be a slug, or a string consisting of only
658-
# ASCII letters, numbers, underscores or hyphens.
659-
# Lets replace all "." with "_"
660-
return s.replace(".", "_")
661-
662656
def __init__(
663657
self,
664658
patchwork: Patchwork,
@@ -1277,12 +1271,12 @@ async def sync_checks(self, pr: PullRequest, series: Series) -> None:
12771271
# cached state).
12781272
pr.update()
12791273
# if it's merge conflict - report failure
1280-
ctx = BranchWorker.slugify_context(f"{CI_DESCRIPTION}-{self.repo_branch}")
1274+
ctx_prefix = slugify_check_context(f"{self.repo_branch}")
12811275
if pr_has_label(pr, MERGE_CONFLICT_LABEL):
12821276
await series.set_check(
12831277
status=Status.CONFLICT,
12841278
target_url=pr.html_url,
1285-
context=f"{ctx}-PR",
1279+
context=f"{ctx_prefix}_PR",
12861280
description=MERGE_CONFLICT_LABEL,
12871281
)
12881282
await self.evaluate_ci_result(Status.CONFLICT, series, pr, [])
@@ -1344,17 +1338,17 @@ async def sync_checks(self, pr: PullRequest, series: Series) -> None:
13441338
self.submit_pr_summary(
13451339
series=series,
13461340
status=status,
1347-
context_name=ctx,
1341+
context_name=ctx_prefix,
13481342
target_url=pr.html_url,
13491343
)
13501344
] + [
13511345
series.set_check(
13521346
status=gh_conclusion_to_status(job.conclusion),
13531347
target_url=job.html_url,
1354-
context=BranchWorker.slugify_context(f"{ctx}-{CI_VMTEST_NAME}-{idx}"),
1348+
context=slugify_check_context(f"{ctx_prefix}_{job.name}"),
13551349
description=f"Logs for {job.name}",
13561350
)
1357-
for idx, job in enumerate(jobs)
1351+
for job in jobs
13581352
]
13591353
await asyncio.gather(*tasks)
13601354

kernel_patches_daemon/patchwork.py

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,22 @@
9696
)
9797

9898

99+
CHECK_CONTEXT_NOT_ALLOWED_CHARS_RE: Final[re.Pattern] = re.compile(r"[^-a-zA-Z0-9_]+")
100+
MULTIPLE_UNDERSCORES_RE: Final[re.Pattern] = re.compile(r"_+")
101+
102+
103+
def slugify_check_context(check_context: str) -> str:
104+
"""
105+
Slugify provided string, so that it can be used as a "context" field of a patchwork check.
106+
See: https://github.com/getpatchwork/patchwork/blob/v3.1.3/docs/api/schemas/latest/patchwork.yaml#L1579
107+
"""
108+
result = CHECK_CONTEXT_NOT_ALLOWED_CHARS_RE.sub("_", check_context)
109+
result = MULTIPLE_UNDERSCORES_RE.sub("_", result)
110+
result = result.strip("_")
111+
112+
return result
113+
114+
99115
## Request Tracing
100116
class TraceContext(SimpleNamespace):
101117
"""
@@ -700,11 +716,11 @@ async def post_check_for_patch_id(
700716
)
701717
return None
702718

703-
logger.info(
719+
logger.debug(
704720
f"Updating patch {patch_id} check, current state: '{check.get('state')}', new state: '{new_state}'"
705721
)
706722
except ValueError:
707-
logger.info(f"Setting patch {patch_id} check to '{new_state}' state")
723+
logger.debug(f"Setting patch {patch_id} check to '{new_state}' state")
708724

709725
return await self.__try_post(
710726
f"patches/{patch_id}/checks/",

tests/test_github_sync.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -398,7 +398,7 @@ async def test_sync_patches_pr_summary_success(self, m: aioresponses) -> None:
398398
post_data = post_calls[0].kwargs["data"]
399399
expected_post_data = {
400400
"target_url": "https://github.com/org/repo/pull/98765",
401-
"context": "vmtest-test-bpf-next-PR",
401+
"context": "test-bpf-next-PR",
402402
"description": "PR summary",
403403
"state": "success",
404404
}

0 commit comments

Comments
 (0)