Skip to content

Commit 015b5e1

Browse files
HadhemiDDclaude
andauthored
Model GitHub API enum fields as StrEnum in the async client (DataDog#24316)
* Model GitHub API enum fields as StrEnum in the async client Convert response-model fields the GitHub OpenAPI description (pinned to GITHUB_API_VERSION 2022-11-28) declares as an enum into StrEnums, per the github_async guidelines: - PullRequestState (open, closed) for PullRequest.state - WorkflowJobStatus / WorkflowJobConclusion for WorkflowJob - JobStepStatus (3-value) for JobStep.status (JobStep.conclusion has no declared enum, kept str | None) - CheckRunStatus / CheckRunConclusion for CheckRun WorkflowRun.status/conclusion and GitHubUser.type declare no enum in the description, so they stay free-form strings. Add WorkflowRun.is_completed helper, retype update_check_run's conclusion param, and cover the new field types in the async client unit tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Add changelog entry Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Sort model re-exports (ruff isort) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Change changelog entry type to fixed Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Use auto() for StrEnum values in the async GitHub models Every member's declared value equals its lowercased name, and StrEnum's auto() generates exactly that, so this is behavior-preserving. Also document in the folder AGENTS.md when auto() applies and when values must stay explicit. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Add stale conclusion; refactor github_async client tests to endpoint-level coverage - Add STALE to CheckRunConclusion: GitHub can return conclusion="stale" on a check run (present in the update-a-check-run request enum/docs though omitted from the response schema), so the model must accept it. - Drop the model-only tests that just exercised Pydantic; move enum-type coverage into the endpoint tests (parametrized over the API response), where a broken model breaks a real client test. - Standardize every payload-builder helper on the **overrides pattern and merge the duplicate workflow-job builder into one. - Fill endpoint-coverage gaps: get_pull_request (success/headers/http_error) and create/update_check_run http-error tests; fold is_completed into a parametrized test_get_workflow_run_success. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Drop leading underscores from test helpers in test_github_async.py Non-public module-level helpers and constants don't need a privacy underscore prefix; it's noise. Applies the convention from PR DataDog#24253 review to this branch's test file. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Give test payload builders explicit params instead of bare **overrides Each fixture builder now names the fields it expects and keeps a trailing **extra for injected/unknown fields, so callers can see the accepted parameters from the signature without scrolling to the definition. Addresses review feedback on PR DataDog#24316; call sites are unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent c54401a commit 015b5e1

8 files changed

Lines changed: 554 additions & 191 deletions

File tree

ddev/changelog.d/24316.fixed

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Retype async GitHub client response-model enum fields (PR state, workflow-job and check-run status/conclusion) as StrEnums.

ddev/src/ddev/utils/github_async/AGENTS.md

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,12 +25,28 @@ is real and must be checked per field:
2525
a `StrEnum` must list all six, not a guessed subset.
2626

2727
```python
28-
from enum import StrEnum
28+
from enum import StrEnum, auto
2929

3030

3131
class PullRequestState(StrEnum):
32-
OPEN = "open"
33-
CLOSED = "closed"
32+
OPEN = auto()
33+
CLOSED = auto()
34+
```
35+
36+
Prefer `auto()` for the values: on a `StrEnum` it generates each value as the
37+
**lowercased member name**, with underscores preserved (`IN_PROGRESS` becomes
38+
`"in_progress"`). Use it only when every declared enum value is exactly
39+
`member_name.lower()`. When the API's value differs — a different case, hyphens,
40+
or any form that isn't the lowercased name — spell the value out explicitly
41+
instead of forcing `auto()`. For example `pull-request.author_association`
42+
declares UPPERCASE values, so `auto()` would wrongly lowercase them and each must
43+
be written out:
44+
45+
```python
46+
class AuthorAssociation(StrEnum):
47+
OWNER = "OWNER"
48+
COLLABORATOR = "COLLABORATOR"
49+
# ...
3450
```
3551

3652
A plain `str` field for a declared enum hides the contract and lets invalid

ddev/src/ddev/utils/github_async/client.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
from .models import (
2121
ArtifactsList,
2222
CheckRun,
23+
CheckRunConclusion,
2324
IssueComment,
2425
Label,
2526
PullRequest,
@@ -548,7 +549,7 @@ async def update_check_run(
548549
repo: str,
549550
check_run_id: int,
550551
status: Literal["queued", "in_progress", "completed"] | None = None,
551-
conclusion: str | None = None,
552+
conclusion: CheckRunConclusion | None = None,
552553
details_url: str | None = None,
553554
output: dict[str, Any] | None = None,
554555
timeout: float | None = None,

ddev/src/ddev/utils/github_async/models/__init__.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,18 +26,24 @@
2626
# break the lazy-loading guarantee. The `X as X` aliases mark these as
2727
# explicit re-exports for linters.
2828
from .check_run import CheckRun as CheckRun
29+
from .check_run import CheckRunConclusion as CheckRunConclusion
30+
from .check_run import CheckRunStatus as CheckRunStatus
2931
from .comment import IssueComment as IssueComment
3032
from .comment import PullRequestReviewComment as PullRequestReviewComment
3133
from .label import Label as Label
3234
from .pull_request import PullRequest as PullRequest
3335
from .pull_request import PullRequestRef as PullRequestRef
36+
from .pull_request import PullRequestState as PullRequestState
3437
from .user import GitHubUser as GitHubUser
3538
from .workflow import Artifact as Artifact
3639
from .workflow import ArtifactsList as ArtifactsList
3740
from .workflow import JobStep as JobStep
41+
from .workflow import JobStepStatus as JobStepStatus
3842
from .workflow import WorkflowDispatchResult as WorkflowDispatchResult
3943
from .workflow import WorkflowJob as WorkflowJob
44+
from .workflow import WorkflowJobConclusion as WorkflowJobConclusion
4045
from .workflow import WorkflowJobsList as WorkflowJobsList
46+
from .workflow import WorkflowJobStatus as WorkflowJobStatus
4147
from .workflow import WorkflowRun as WorkflowRun
4248

4349
# Map of exported attribute name -> submodule (relative to this package) that
@@ -46,15 +52,21 @@
4652
'Artifact': 'workflow',
4753
'ArtifactsList': 'workflow',
4854
'CheckRun': 'check_run',
55+
'CheckRunConclusion': 'check_run',
56+
'CheckRunStatus': 'check_run',
4957
'GitHubUser': 'user',
5058
'IssueComment': 'comment',
5159
'JobStep': 'workflow',
60+
'JobStepStatus': 'workflow',
5261
'Label': 'label',
5362
'PullRequest': 'pull_request',
5463
'PullRequestRef': 'pull_request',
5564
'PullRequestReviewComment': 'comment',
65+
'PullRequestState': 'pull_request',
5666
'WorkflowDispatchResult': 'workflow',
5767
'WorkflowJob': 'workflow',
68+
'WorkflowJobConclusion': 'workflow',
69+
'WorkflowJobStatus': 'workflow',
5870
'WorkflowJobsList': 'workflow',
5971
'WorkflowRun': 'workflow',
6072
}

ddev/src/ddev/utils/github_async/models/check_run.py

Lines changed: 49 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,17 +5,63 @@
55

66
from __future__ import annotations
77

8+
from enum import StrEnum, auto
9+
810
from pydantic import BaseModel, ConfigDict
911

1012

13+
class CheckRunStatus(StrEnum):
14+
"""The status of a check run.
15+
16+
The `check-run` schema declares `status` as
17+
`enum: [queued, in_progress, completed, waiting, requested, pending]`.
18+
Reference:
19+
https://docs.github.com/en/rest/checks/runs#get-a-check-run
20+
"""
21+
22+
QUEUED = auto()
23+
IN_PROGRESS = auto()
24+
COMPLETED = auto()
25+
WAITING = auto()
26+
REQUESTED = auto()
27+
PENDING = auto()
28+
29+
30+
class CheckRunConclusion(StrEnum):
31+
"""The conclusion of a check run.
32+
33+
The `check-run` response schema declares `conclusion` as a nullable
34+
`enum: [success, failure, neutral, cancelled, skipped, timed_out, action_required]`.
35+
`stale` is added on top of those: it is a valid conclusion that only GitHub can
36+
set (present in the update-a-check-run request enum and the docs, though the
37+
response schema omits it), so a real response can carry it and the model must
38+
accept it.
39+
Reference:
40+
https://docs.github.com/en/rest/checks/runs#update-a-check-run
41+
"""
42+
43+
SUCCESS = auto()
44+
FAILURE = auto()
45+
NEUTRAL = auto()
46+
CANCELLED = auto()
47+
SKIPPED = auto()
48+
TIMED_OUT = auto()
49+
ACTION_REQUIRED = auto()
50+
STALE = auto()
51+
52+
1153
class CheckRun(BaseModel):
12-
"""A GitHub Checks API check run."""
54+
"""A GitHub Checks API check run.
55+
56+
Field reference:
57+
https://docs.github.com/en/rest/checks/runs#get-a-check-run
58+
"""
1359

1460
model_config = ConfigDict(extra="ignore")
1561

1662
id: int
1763
name: str
18-
status: str
19-
conclusion: str | None
64+
status: CheckRunStatus
65+
conclusion: CheckRunConclusion | None
2066
html_url: str | None
2167
head_sha: str

ddev/src/ddev/utils/github_async/models/pull_request.py

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,26 @@
55

66
from __future__ import annotations
77

8+
from enum import StrEnum, auto
9+
810
from pydantic import BaseModel, ConfigDict, Field
911

1012
from .label import Label
1113
from .user import GitHubUser
1214

1315

16+
class PullRequestState(StrEnum):
17+
"""The state of a pull request.
18+
19+
The `pull-request` schema declares `state` as `enum: [open, closed]`.
20+
Reference:
21+
https://docs.github.com/en/rest/pulls/pulls#get-a-pull-request
22+
"""
23+
24+
OPEN = auto()
25+
CLOSED = auto()
26+
27+
1428
class PullRequestRef(BaseModel):
1529
"""A head or base branch reference on a pull request.
1630
@@ -50,7 +64,7 @@ class PullRequest(BaseModel):
5064
patch_url: str | None = None
5165

5266
# State
53-
state: str | None = None # 'open' or 'closed'
67+
state: PullRequestState | None = None
5468
draft: bool = False
5569
merged: bool | None = None
5670
locked: bool = False

ddev/src/ddev/utils/github_async/models/workflow.py

Lines changed: 79 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,71 @@
55

66
from __future__ import annotations
77

8+
from enum import StrEnum, auto
9+
810
from pydantic import BaseModel, ConfigDict, Field
911

1012

13+
class WorkflowJobStatus(StrEnum):
14+
"""The status of a workflow job.
15+
16+
The `job` schema declares `status` as
17+
`enum: [queued, in_progress, completed, waiting, requested, pending]`.
18+
Reference:
19+
https://docs.github.com/en/rest/actions/workflow-jobs#get-a-job-for-a-workflow-run
20+
"""
21+
22+
QUEUED = auto()
23+
IN_PROGRESS = auto()
24+
COMPLETED = auto()
25+
WAITING = auto()
26+
REQUESTED = auto()
27+
PENDING = auto()
28+
29+
30+
class WorkflowJobConclusion(StrEnum):
31+
"""The conclusion of a workflow job.
32+
33+
The `job` schema declares `conclusion` as a nullable
34+
`enum: [success, failure, neutral, cancelled, skipped, timed_out, action_required]`.
35+
Reference:
36+
https://docs.github.com/en/rest/actions/workflow-jobs#get-a-job-for-a-workflow-run
37+
"""
38+
39+
SUCCESS = auto()
40+
FAILURE = auto()
41+
NEUTRAL = auto()
42+
CANCELLED = auto()
43+
SKIPPED = auto()
44+
TIMED_OUT = auto()
45+
ACTION_REQUIRED = auto()
46+
47+
48+
class JobStepStatus(StrEnum):
49+
"""The status of a single step within a workflow job.
50+
51+
The `job` schema's `steps` items declare `status` as
52+
`enum: [queued, in_progress, completed]` (a narrower set than the job's own
53+
status). Their `conclusion` is a nullable string with no `enum`, so it stays
54+
a plain `str`.
55+
Reference:
56+
https://docs.github.com/en/rest/actions/workflow-jobs#get-a-job-for-a-workflow-run
57+
"""
58+
59+
QUEUED = auto()
60+
IN_PROGRESS = auto()
61+
COMPLETED = auto()
62+
63+
1164
class WorkflowRun(BaseModel):
12-
"""A GitHub Actions workflow run."""
65+
"""A GitHub Actions workflow run.
66+
67+
The `workflow-run` schema declares `status` and `conclusion` as plain
68+
nullable strings with no `enum`, so they are intentionally kept as free-form
69+
strings rather than modeled as a StrEnum.
70+
Reference:
71+
https://docs.github.com/en/rest/actions/workflow-runs#get-a-workflow-run
72+
"""
1373

1474
model_config = ConfigDict(extra="ignore")
1575

@@ -21,6 +81,11 @@ class WorkflowRun(BaseModel):
2181
created_at: str | None = None
2282
updated_at: str | None = None
2383

84+
@property
85+
def is_completed(self) -> bool:
86+
"""Whether the run has finished (``status == "completed"``)."""
87+
return self.status == "completed"
88+
2489

2590
class WorkflowDispatchResult(BaseModel):
2691
"""Run metadata returned by `POST /actions/workflows/{id}/dispatches` when `return_run_details=True`."""
@@ -55,26 +120,34 @@ class ArtifactsList(BaseModel):
55120

56121

57122
class JobStep(BaseModel):
58-
"""A single step within a GitHub Actions job."""
123+
"""A single step within a GitHub Actions job.
124+
125+
Field reference:
126+
https://docs.github.com/en/rest/actions/workflow-jobs#get-a-job-for-a-workflow-run
127+
"""
59128

60129
model_config = ConfigDict(extra="ignore")
61130

62131
name: str
63-
status: str
132+
status: JobStepStatus
64133
conclusion: str | None = None
65134
number: int | None = None
66135

67136

68137
class WorkflowJob(BaseModel):
69-
"""A single job within a GitHub Actions workflow run."""
138+
"""A single job within a GitHub Actions workflow run.
139+
140+
Field reference:
141+
https://docs.github.com/en/rest/actions/workflow-jobs#get-a-job-for-a-workflow-run
142+
"""
70143

71144
model_config = ConfigDict(extra="ignore")
72145

73146
id: int
74147
run_id: int
75148
name: str
76-
status: str
77-
conclusion: str | None = None
149+
status: WorkflowJobStatus
150+
conclusion: WorkflowJobConclusion | None = None
78151
html_url: str | None = None
79152
steps: list[JobStep] = Field(default_factory=list)
80153

0 commit comments

Comments
 (0)