Restructure github_async into a lazy-loaded package#23685
Conversation
cbef667 to
e33bd03
Compare
This stack of pull requests is managed by Graphite. Learn more about stacking. |
e33bd03 to
579edab
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 579edabe87
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| _MODULE_BY_NAME: dict[str, str] = { | ||
| 'AsyncGitHubClient': 'client', | ||
| 'async_github_client': 'client', | ||
| 'GITHUB_API_VERSION': 'client', | ||
| 'DEFAULT_BASE_URL': 'client', | ||
| 'GitHubResponse': 'client', | ||
| 'PaginationData': 'client', | ||
| } |
There was a problem hiding this comment.
Restore top-level model re-exports
This map now omits the model classes that were exported by the old ddev.utils.github_async module, so existing callers using imports such as from ddev.utils.github_async import WorkflowRun or ArtifactsList will fail with ImportError after this refactor even though the changelog describes a package restructure rather than a breaking API change. Add the model names here (pointing at the appropriate models.* modules) or otherwise preserve the old top-level imports while keeping the lazy loading behavior.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
There are no existing callers yet, that is the point of moving it now. This was just introduced and we want to set it in a way that we can maintain it for easy imports without too long import lines while doing it lazily so we only import what is needed.
579edab to
5729ee2
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files🚀 New features to boost your workflow:
|
🎉 All green!❄️ No new flaky tests detected 🎯 Code Coverage (details) 🔗 Commit SHA: 1c9e7d3 | Docs | Datadog PR Page | Give us feedback! |
99fa9d0 to
2a381c6
Compare
Splits the single-file `ddev/utils/github_async.py` into a package with the client and HTTP-shape primitives in `client.py` and one model per submodule under `models/`. Both `__init__.py` files use PEP 562 module-level `__getattr__` so importing one symbol only loads the submodule that defines it -- in particular `from ddev.utils.github_async.models import PullRequest` does not pull in the workflow or comment models. Adds two new endpoints used by upcoming work: - `AsyncGitHubClient.create_pull_request(owner, repo, title, head, base, body, draft)` - `AsyncGitHubClient.add_labels_to_issue(owner, repo, issue_number, labels)` Expands the `PullRequest` model with the typical fields callers need (id, state, draft, title, body, user, assignees, requested_reviewers, labels, created_at/updated_at/closed_at/merged_at, head, base) plus three small sub-models (`GitHubUser`, `Label`, `PullRequestRef`). Only `number` and `html_url` are required; the rest default to None/[] so partial payloads parse fine and `extra='ignore'` keeps the schema forward-compatible. Adds `FakeAsyncGitHubClient` and the `fake_async_github` pytest fixture in `tests/helpers/github_async.py`. The fake records every call and offers `mock_response(method, response, /, *, once=False, **match_kwargs)` for stubbing replies. Responses can be `BaseException` instances (raised), `GitHubResponse` instances (passed through), or inner data (auto-wrapped). `once=True` adds to a per-method FIFO queue so tests can model retry sequences. Sticky mocks (no `once`) match the most-recent registration. `assert_called_with` / `assert_called_once_with` perform strict-equality checks on kwargs; `assert_all_responses_consumed()` asserts the one-shot queue was drained.
2a381c6 to
1c9e7d3
Compare
Validation ReportAll 20 validations passed. Show details
|
Splits the single-file `ddev/utils/github_async.py` into a package with the client and HTTP-shape primitives in `client.py` and one model per submodule under `models/`. Both `__init__.py` files use PEP 562 module-level `__getattr__` so importing one symbol only loads the submodule that defines it -- in particular `from ddev.utils.github_async.models import PullRequest` does not pull in the workflow or comment models. Adds two new endpoints used by upcoming work: - `AsyncGitHubClient.create_pull_request(owner, repo, title, head, base, body, draft)` - `AsyncGitHubClient.add_labels_to_issue(owner, repo, issue_number, labels)` Expands the `PullRequest` model with the typical fields callers need (id, state, draft, title, body, user, assignees, requested_reviewers, labels, created_at/updated_at/closed_at/merged_at, head, base) plus three small sub-models (`GitHubUser`, `Label`, `PullRequestRef`). Only `number` and `html_url` are required; the rest default to None/[] so partial payloads parse fine and `extra='ignore'` keeps the schema forward-compatible. Adds `FakeAsyncGitHubClient` and the `fake_async_github` pytest fixture in `tests/helpers/github_async.py`. The fake records every call and offers `mock_response(method, response, /, *, once=False, **match_kwargs)` for stubbing replies. Responses can be `BaseException` instances (raised), `GitHubResponse` instances (passed through), or inner data (auto-wrapped). `once=True` adds to a per-method FIFO queue so tests can model retry sequences. Sticky mocks (no `once`) match the most-recent registration. `assert_called_with` / `assert_called_once_with` perform strict-equality checks on kwargs; `assert_all_responses_consumed()` asserts the one-shot queue was drained.

What does this PR do?
Restructures
ddev.utils.github_asyncfrom a single file into a package with the client inclient.pyand one model per submodule undermodels/. Both__init__.pyfiles use PEP 562__getattr__, so e.g.from ddev.utils.github_async.models import PullRequestonly loadspull_request.py(plus itsuser/labeldependencies), not every model.Adds two new endpoints needed by upcoming work, expands the
PullRequestmodel with the fields you typically need when inspecting a PR (state, draft, title/body, user/assignees, labels, branches, timestamps), and introduces aFakeAsyncGitHubClienttest helper with amock_responseAPI supporting sticky replies, one-shot FIFO queues, partial-kwarg matching, and exception responses.Motivation
Sets up the async-client surface and test infrastructure that a stacked PR on top will use to add a new
ddev release port-commitcommand. Lands the refactor on its own so review can focus on the client/test-fixture shape independent of feature code.Review checklist (to be filled by reviewers)
qa/skip-qalabel if the PR doesn't need to be tested during QA.backport/<branch-name>label to the PR and it will automatically open a backport PR once this one is merged