Swap mesos_master and control_m test exceptions to agnostic http_exceptions hierarchy#24212
Conversation
…ptions Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
This new production import relies on the newer datadog_checks.base.utils.http_exceptions module, but mesos_master/pyproject.toml:31 still allows datadog-checks-base>=37.33.0. In standalone installs that resolve one of those still-allowed older base versions, importing datadog_checks.mesos_master will fail before the check can run; please raise the minimum base dependency alongside this import.
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 290af0a8a2
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Validation Report
Run Passed validations (20)
|
4ee19f0
into
mwdd146980/httpx-migration-base
What does this PR do?
Moves the
mesos_masterandcontrol_mtest suites offrequests.exceptionsand onto the backend-agnostichttp_exceptionshierarchy, so their assertions no longer name a backend-specific exception type and stay green once these integrations move tohttpx2.requests'
HTTPErroris a status error, so it maps toHTTPStatusError(the leaf), not the agnosticHTTPErrorroot. Both integrations are safe to swap now because each injects the exception through a mock and production already catches the agnostic type (mesos_master.py:187) or catchesExceptionand re-raises (control_m/check.py).Motivation
Part of the
requeststohttpx2migration. Decoupling these assertions fromrequests.exceptionslets the suites pass under either backend, ahead of the per-integration flip.Verification
ddev --no-interactive test mesos_master: 17 passed, 1 e2e skipped.ddev --no-interactive test control_m: 28 passed.ddev test -fs mesos_master control_m: clean.Review checklist (to be filled by reviewers)
qa/requiredif this PR needs QA validation, orqa/skip-qaif it does not. Exactly one of the two is required.backport/<branch-name>label to the PR and it will automatically open a backport PR once this one is merged