refactor(transport): add the seams the httpx swap turns on - #18
Merged
Conversation
Plan 2 of the 0.4.0 rewrite. Removes every httpx incompatibility while still running on requests, so the transport swap becomes a mechanical change rather than a risky one. No behaviour change; the suite is green throughout. * `.reason` is now read through `response_reason()` in `_http.py`. requests spells it `Response.reason`; httpx spells it `reason_phrase` and has no `reason` at all. * Session construction is centralised in `build_http_session(verify_ssl=)` and `build_client_resources` takes an optional `session_factory`. Two things fall out: the SSL policy is applied *at construction*, which httpx requires (it reads `verify` only in `Client.__init__` and silently ignores a later assignment, leaving verification ON when the caller asked for it OFF); and tests can inject a stubbed transport without patching module globals. The factory is a Protocol rather than a Callable alias so the keyword-only `verify_ssl` is part of the contract. `GLPIV1Session` now builds its session the same way. * Both dynamic verb dispatches are gone. `_transport.py` used `getattr(self._session, method)` and `_v1_session.py` used `getattr(self._http, method.lower())`; both now call `session.request(VERB, url, ...)`, the one call shape requests and httpx agree on. Worth noting the getattr in `_transport.py` was also hiding an untyped call: replacing it made mypy reject the `**kwargs: object` passthrough that had previously type-checked only because getattr returns Any. * New `test_method_invocation.py` drives all 85 public methods against a stub installed at the `session.request` seam, so URL building, headers, parameter normalisation, response validation and model parsing all really run. It asserts each method reaches the transport, that the async client mirrors the surface, and that the surface has not shrunk. Three methods are exempted with stated reasons. This is the regression net for plans 3-5, where the failure mode is a method that stops dispatching entirely rather than one that returns a wrong payload -- a per-endpoint suite only catches that where a test happens to exist. A companion test pins the seam by stubbing only `session.get` and asserting it is NOT used, so a regression to per-verb dispatch fails. Two items from the plan needed no work, verified rather than assumed: every re-exporting `__init__.py` already declares `__all__`, and `_config.py` at 249 lines did not warrant splitting. 562 tests pass (from 473), mypy strict clean, ruff clean, coverage 96.90%. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #18 +/- ##
==========================================
+ Coverage 96.69% 96.89% +0.20%
==========================================
Files 79 79
Lines 2634 2644 +10
==========================================
+ Hits 2547 2562 +15
+ Misses 87 82 -5 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Plan 2 of the 0.4.0 rewrite. Removes every httpx incompatibility while still running on requests, so the transport swap becomes a mechanical change rather than a risky one. No behaviour change; the suite is green throughout.
.reasonis now read throughresponse_reason()in_http.py. requests spells itResponse.reason; httpx spells itreason_phraseand has noreasonat all.Session construction is centralised in
build_http_session(verify_ssl=)andbuild_client_resourcestakes an optionalsession_factory. Two things fall out: the SSL policy is applied at construction, which httpx requires (it readsverifyonly inClient.__init__and silently ignores a later assignment, leaving verification ON when the caller asked for it OFF); and tests can inject a stubbed transport without patching module globals. The factory is a Protocol rather than a Callable alias so the keyword-onlyverify_sslis part of the contract.GLPIV1Sessionnow builds its session the same way.Both dynamic verb dispatches are gone.
_transport.pyusedgetattr(self._session, method)and_v1_session.pyusedgetattr(self._http, method.lower()); both now callsession.request(VERB, url, ...), the one call shape requests and httpx agree on. Worth noting the getattr in_transport.pywas also hiding an untyped call: replacing it made mypy reject the**kwargs: objectpassthrough that had previously type-checked only because getattr returns Any.New
test_method_invocation.pydrives all 85 public methods against a stub installed at thesession.requestseam, so URL building, headers, parameter normalisation, response validation and model parsing all really run. It asserts each method reaches the transport, that the async client mirrors the surface, and that the surface has not shrunk. Three methods are exempted with stated reasons. This is the regression net for plans 3-5, where the failure mode is a method that stops dispatching entirely rather than one that returns a wrong payload -- a per-endpoint suite only catches that where a test happens to exist.A companion test pins the seam by stubbing only
session.getand asserting it is NOT used, so a regression to per-verb dispatch fails.Two items from the plan needed no work, verified rather than assumed: every re-exporting
__init__.pyalready declares__all__, and_config.pyat 249 lines did not warrant splitting.562 tests pass (from 473), mypy strict clean, ruff clean, coverage 96.90%.