feat(http): migrate HTTP client from requests to httpx #118
Conversation
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
- Replace `requests` with `httpx` throughout the MpesaHttpClient - Update retry logic to handle `httpx` exceptions (TimeoutException, ConnectError) - Refactor session management to use `httpx.Client` - Adjust error handling to use `response.is_success` instead of `response.ok` - Update unit tests to mock `httpx.Client` methods and use `httpx.Response` - Fix integration test to use `httpx.Client` for callback validation - Loosen `raw_response` type in `MpesaError` to support non-dict payloads This change enables async support readiness, better timeout control, and modern HTTP/2 capabilities while maintaining the same public API.
Contributor
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughMigrates internal HTTP usage from requests to httpx, broadens MpesaError.raw_response type to Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In `@tests/unit/http_client/test_mpesa_http_client.py`:
- Around line 114-121: The assertions are placed inside the with
pytest.raises(MpesaApiException) block so they never run; move the mock and
exception assertions outside the context manager: keep the patch of
client._raw_post with side_effect httpx.ConnectError and call
client.post("/test", json={"a": 1}, headers={"h": "v"}) inside with
pytest.raises(MpesaApiException) as exc, then after that block assert
mock_raw_post.call_count == 3 and assert exc.value.error.error_code ==
"CONNECTION_ERROR" to verify retry attempts and the raised MpesaApiException.
- Around line 148-159: The assertions in test_get_json_decode_error are placed
inside the with pytest.raises(MpesaApiException) context so they never run;
update the test_get_json_decode_error to keep the mock setup (patching
client._raw_get and setting mock_response.json to raise ValueError) and call
client.get("/fail") inside the with pytest.raises(MpesaApiException) as exc
block, but move the assertions (checking exc.value.error.error_code ==
"HTTP_500" and that "Internal Server Error" is in exc.value.error.error_message)
to immediately after the with block so they execute, ensuring the test still
validates client.get error handling.
- Around line 75-79: The assertions for the raised MpesaApiException are
currently inside the with pytest.raises(MpesaApiException) context and never
run; move the two asserts so they execute after the context manager exits.
Specifically, keep the call that should raise (client.post("/fail", json={},
headers={})) inside the with pytest.raises(MpesaApiException) as exc: block,
then immediately after that block assert on exc.value.error.error_code ==
"HTTP_500" and that "Internal Server Error" is in exc.value.error.error_message
to validate the exception details.
- Around line 186-195: The test test_get_fails_after_max_retries has assertions
placed inside the with pytest.raises(MpesaApiException) context so they never
run; move the assertions that check mock_raw_get.call_count and
exc.value.error.error_code to after the with block and capture the raised
exception via the context manager (the exc variable) so you can assert on its
contents—update references in the test to ensure mock_raw_get is patched on
client._raw_get and that the side_effect remains httpx.TimeoutException("Read
timed out.") while asserting call_count == 3 and exc.value.error.error_code ==
"REQUEST_TIMEOUT" outside the with block.
- Around line 53-63: The mock for client._raw_post is exiting before the call
under test, causing the real method to be invoked; in test_post_http_error
ensure the patch.object context remains active when calling client.post by
moving the client.post(...) invocation (and its pytest.raises block) inside the
with patch.object(client, "_raw_post") as mock_raw_post: block (or convert to a
decorator-based patch) so mock_raw_post.return_value is used; keep the
assertions on exc.value.error.* after the call within the same context and
reference test_post_http_error, client._raw_post, client.post, and
MpesaApiException when applying the change.
- Around line 44-50: The tests create httpx.Response objects using a nonexistent
json= kwarg which raises TypeError; update each Response(...) construction
(e.g., the mock_response used with patch.object(client, "_raw_post") / variable
mock_raw_post) to pass content=json.dumps({...}).encode() and appropriate
status_code (and headers if needed) or alternatively use pytest-httpx test
helpers; ensure you import json where used and fix all other occurrences in this
file (the other Response(...) instances around the same tests) to use content
rather than json.
🧹 Nitpick comments (1)
mpesakit/http_client/mpesa_http_client.py (1)
184-200: GET method has hardcoded timeout while POST accepts a configurable timeout.
_raw_getuses a hardcodedtimeout=10(lines 196, 200), but_raw_postaccepts atimeoutparameter. Consider adding atimeoutparameter to_raw_getandgetfor consistency.♻️ Proposed refactor
def _raw_get( self, url: str, params: Optional[Dict[str, Any]] = None, headers: Optional[Dict[str, str]] = None, + timeout: int = 10, ) -> httpx.Response: """Low-level GET request - may raise httpx exceptions.""" if headers is None: headers = {} full_url = urljoin(self.base_url, url) if self._client: return self._client.get( - full_url, params=params, headers=headers, timeout=10 + full_url, params=params, headers=headers, timeout=timeout ) else: with httpx.Client() as client: - return client.get(full_url, params=params, headers=headers, timeout=10) + return client.get(full_url, params=params, headers=headers, timeout=timeout)And similarly update the public
getmethod signature.
Update unit tests for retry exhaustion scenarios to mock httpx.Client methods instead of _raw_* methods, ensuring the retry decorator is properly exercised. Also fix indentation and assertion alignment in error-handling tests.
Expose `timeout` argument in both public `get()` and internal `_raw_get()` methods to match the POST interface, enabling consistent timeout control across all HTTP operations.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
mpesakit/http_client/mpesa_http_client.py (2)
212-218: Docstring missingtimeoutparameter (lint failure).CI reports D417 for the
getdocstring. Add the missing timeout description.✏️ Docstring fix
Args: url (str): The URL path for the request. params (Optional[Dict[str, Any]]): The URL parameters. headers (Optional[Dict[str, str]]): The HTTP headers. + timeout (int): The timeout for the request in seconds.
105-119:trust_envis ignored whenuse_session=False.The non-session path creates ephemeral
httpx.Client()instances without passingtrust_env, so the flag is ineffective and will silently use httpx defaults regardless of user intent. Storetrust_envas an instance variable and reuse it consistently across all Client instantiations.🔧 Proposed fix
class MpesaHttpClient(HttpClient): """A client for making HTTP requests to the M-Pesa API.""" base_url: str _client: Optional[httpx.Client] = None + _trust_env: bool def __init__( self, env: str = "sandbox", use_session: bool = False, trust_env: bool = True ): @@ """ self.base_url = self._resolve_base_url(env) + self._trust_env = trust_env if use_session: - self._client = httpx.Client(trust_env=trust_env) + self._client = httpx.Client(trust_env=self._trust_env) @@ if self._client: return self._client.post( full_url, json=json, headers=headers, timeout=timeout ) else: - with httpx.Client() as client: + with httpx.Client(trust_env=self._trust_env) as client: return client.post( full_url, json=json, headers=headers, timeout=timeout ) @@ if self._client: return self._client.get( full_url, params=params, headers=headers, timeout=timeout ) else: - with httpx.Client() as client: + with httpx.Client(trust_env=self._trust_env) as client: return client.get( full_url, params=params, headers=headers, timeout=timeout )
🧹 Nitpick comments (1)
mpesakit/http_client/mpesa_http_client.py (1)
162-162: Verify Python version forResponse | None.This syntax requires Python ≥3.10. If you still support 3.9 or lower, switch to
Optional[...](or bump the minimum version).🔧 Compatible typing alternative
- response: httpx.Response | None = None + response: Optional[httpx.Response] = None @@ - response: httpx.Response | None = None + response: Optional[httpx.Response] = NoneAlso applies to: 222-222
15 tasks
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.
Description
This PR migrates the
MpesaHttpClientfrom therequestslibrary tohttpx. The primary motivation is to modernize the HTTP layer, gain better timeout handling, prepare for future async support, and align with current Python ecosystem trends. All public APIs remain unchanged—this is a drop-in internal upgrade.The migration includes:
requestsusages with equivalenthttpxcallshttpx.RequestError,TimeoutException, andConnectErrorhttpx.Clientresponse.oktoresponse.is_successhttpxFixes #54
Type of Change
How Has This Been Tested?
httpx.Responseand properly mockhttpx.Client.post/get. Retries, error wrapping, success paths, and JSON parsing failures are all verified.httpx.Clientfor callback validation and remains fully functional in sandbox mode.post()andget()signatures and behaviors are preserved.Checklist
Additional Context
While this change is currently synchronous, it lays the groundwork for optional async support in the future by using
httpx, which supports both sync and async clients with nearly identical APIs. Additionally,httpxprovides more precise timeout controls and native HTTP/2 support, which may improve performance in high-throughput scenarios.The loosening of
raw_responsefromDict[str, Any]toAnyinMpesaErrorensures robustness when dealing with malformed or non-JSON error responses from upstream systems.Summary by CodeRabbit
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.