-
Notifications
You must be signed in to change notification settings - Fork 1
CFT-3212 - Feature/async #9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
de527c3 to
4a4877f
Compare
4a4877f to
841709f
Compare
bacb974 to
e15824c
Compare
cd52e02 to
a733f7c
Compare
a733f7c to
f9db5cf
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Adds an asynchronous HTTP client alongside the existing synchronous client, migrates packaging to pyproject.toml with uv-based workflows, modernizes type hints, and expands documentation and examples to cover async usage and CI changes.
- Introduces AsyncHttpClient with retry, backoff, optional rate limiting, and batch processing.
- Refactors sync client type hints / formatting and replaces setup.py with pyproject.toml plus updated CI workflows.
- Expands README and examples for async usage (some documentation inconsistencies introduced).
Reviewed Changes
Copilot reviewed 15 out of 18 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_async.py | New test suite for AsyncHttpClient (params/header/auth/retry handling; lacks coverage for some new behaviors). |
| src/keboola/http_client/http.py | Type hint modernization, import adjustments, formatting; minor logic unchanged except stylistic edits. |
| src/keboola/http_client/async_client.py | New asynchronous client implementation with retries, backoff, rate limiting, and batch processing. |
| src/keboola/http_client/init.py | Exposes AsyncHttpClient at package root. |
| pyproject.toml | New project metadata replacing setup.py; changes project name and dependency management. |
| flake8.cfg | Updated lint configuration (ignore list and limits). |
| docs/examples/* | Added sync and async usage examples, including batch processing and rate limiting demos. |
| README.md | Extended documentation for async client and reorganized sections (contains inaccuracies). |
| .github/workflows/*.yml | Updated CI to use uv, add Python versions, automate version injection and publishing. |
| setup.py | Removed in favor of pyproject.toml. |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
|
||
| return url | ||
|
|
||
| async def update_auth_header(self, updated_header: dict, overwrite: bool = False): |
Copilot
AI
Oct 6, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Method contains no awaits and performs synchronous mutations only; consider making it a regular def to simplify usage (so callers need not await it) and align with the synchronous client's API.
| async def update_auth_header(self, updated_header: dict, overwrite: bool = False): | |
| def update_auth_header(self, updated_header: dict, overwrite: bool = False): |
| if overwrite is False: | ||
| self._auth_header.update(updated_header) | ||
| else: | ||
| self._auth_header = updated_header |
Copilot
AI
Oct 6, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Method contains no awaits and performs synchronous mutations only; consider making it a regular def to simplify usage (so callers need not await it) and align with the synchronous client's API.
ba0817c to
b6c748f
Compare
The issue was that when mock_request didn't have a return value configured and returned a MagicMock object instead, so when response.raise_for_status() was called, the mock returned a coroutine that was never awaited.
55c4059 to
a7d57e9
Compare
ZdenekSrotyr
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, tests look solid and everything reads well.
soustruh
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, the updated client was thoroughly tested in many components already, the test coverage is also sufficient 👍
Add async HTTP client support
This PR adds an async variant of the HTTP client alongside the existing synchronous implementation.
What's new
AsyncHttpClient: New async client built on
httpxwith the same interface as the sync clientaiolimiterTest coverage: Comprehensive async test suite with proper mock handling
Documentation: Updated README with async examples and usage patterns
Modernization:
setup.pytopyproject.tomluvfor dependency managementdict | Noneinstead ofOptional[Dict])Breaking changes
None - the sync
HttpClientAPI remains unchanged.