-
Notifications
You must be signed in to change notification settings - Fork 0
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
[UPY-7] Support async operations #6
Conversation
WalkthroughThis pull request restructures and enhances the upyloadthing SDK. The changes update documentation to clarify both synchronous and asynchronous client implementations, including new type hints and detailed response model fields. A new abstract base class centralizes shared logic, while the synchronous client now subclasses it. The asynchronous client is introduced with concurrent upload capabilities and robust error handling. Additionally, test suites have been expanded for both client types, examples have been updated to demonstrate multi-file operations, and a new development dependency for asynchronous testing was added. Changes
Sequence Diagram(s)sequenceDiagram
participant Main
participant AsyncUTApi
participant API
Main->>AsyncUTApi: Initialize with token
Main->>AsyncUTApi: get_usage_info()
AsyncUTApi->>API: Send usage info request
API-->>AsyncUTApi: Return usage information
Main->>AsyncUTApi: list_files(limit=5)
AsyncUTApi->>API: Request list files
API-->>AsyncUTApi: Return list of files
Main->>AsyncUTApi: upload_files([png, jpeg])
AsyncUTApi->>API: Concurrently upload files
API-->>AsyncUTApi: Return upload results
Main->>AsyncUTApi: delete_files(file_keys)
AsyncUTApi->>API: Request deletion of files
API-->>AsyncUTApi: Return deletion result
Possibly related PRs
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 1
🧹 Nitpick comments (10)
upyloadthing/client.py (1)
22-67
: Consider persistent HTTP sessions for efficiency.Currently,
_request
spins up a newhttpx.Client()
in a context manager for every request. For high-throughput scenarios, reusing a single session can reduce connection overhead. If usage patterns call for multiple requests in short succession, a persistent session is typically beneficial.def _request( self, method: str, path: str, data: dict | None = None, timeout: float = 30.0, ) -> dict: ... - try: - with httpx.Client() as client: + if not self._session: + self._session = httpx.Client() + try: - response = client.request( + response = self._session.request( method=method, url=url, headers=headers, **request_kwargs ) ...upyloadthing/async_client.py (2)
23-68
: Evaluate whether to separate request creation logic from execution.Currently,
_request
both prepares and executes the HTTP request. For testing, debugging, or advanced usage, consider splitting request creation (URL, headers, etc.) from the actual network call for improved mockability in tests.
129-177
: Potential for advanced retry logic on HTTP failures.Beyond raising exceptions on HTTP failures, you may wish to retry certain transient errors (timeouts, 5xx statuses) to increase reliability when uploading or deleting files in asynchronous workflows.
upyloadthing/base_client.py (2)
118-143
: Extend docstrings with usage examples if needed.Docstrings are sufficient, but consider adding short examples (e.g., snippet usage for typical file uploads) within method docstrings. This can help clarify tricky parameters like
acl
andcontent_disposition
.
144-243
: Potential optimization for larger files.The
_prepare_file_data
function loads entire files into memory when computing size withfile.seek(0, 2)
. For huge files, a chunk-based approach may be necessary, or you might coordinate with the hosting service on streaming.tests/test_client.py (2)
85-85
: Consider adding error case for empty file list.The test verifies successful upload but doesn't check how the API handles an empty file list.
def test_upload_files(respx_mock: respx.MockRouter, ut_api, mock_file): """Test file upload functionality.""" + # Test empty file list + with pytest.raises(ValueError, match="No files provided"): + ut_api.upload_files([]) + upload_response = {
175-199
: Consider adding validation for content disposition.While the test verifies the API call, it doesn't validate that the content disposition is correctly set in the request.
respx_mock.put(url__regex="https://sea2.ingest.uploadthing.com/").mock( - return_value=httpx.Response(200, json=upload_response) + side_effect=lambda request: + httpx.Response(200, json=upload_response) + if "attachment" in request.headers.get("content-disposition", "") + else httpx.Response(400) )tests/test_async_client.py (3)
31-36
: Consider adding cleanup for AsyncUTApi instance.The fixture should handle cleanup of any async resources.
@pytest.fixture -def ut_api(): +async def ut_api(): """Fixture to create an AsyncUTApi instance with mock token.""" options = UTApiOptions(token=MOCK_TOKEN) - return AsyncUTApi(options) + api = AsyncUTApi(options) + yield api + await api.close() # Clean up any async resources
69-103
: Consider testing concurrent uploads.While the test verifies basic upload functionality, it doesn't test concurrent uploads which is a key feature of async APIs.
+@pytest.mark.asyncio +@respx.mock +async def test_concurrent_uploads(respx_mock: respx.MockRouter, ut_api): + """Test concurrent file uploads.""" + files = [BytesIO(f"content{i}".encode()) for i in range(5)] + for i, f in enumerate(files): + f.name = f"test{i}.txt" + + upload_response = { + "fileHash": "hash", + "url": "url", + "ufsUrl": "ufs_url", + "appUrl": "app_url", + } + + respx_mock.put(url__regex="https://sea2.ingest.uploadthing.com/").mock( + return_value=httpx.Response(200, json=upload_response) + ) + + results = await asyncio.gather( + *[ut_api.upload_files([f]) for f in files] + ) + assert len(results) == 5 + assert all(isinstance(r[0], UploadResult) for r in results)
266-276
: Consider testing timeout scenarios.The error handling test should include timeout scenarios which are particularly important for async operations.
+@pytest.mark.asyncio +@respx.mock +async def test_request_with_timeout(respx_mock: respx.MockRouter, ut_api): + """Test handling of request timeouts.""" + respx_mock.get(f"{API_URL}/test").mock(side_effect=httpx.TimeoutException) + + with pytest.raises(httpx.TimeoutException): + await ut_api._request("GET", "/test")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
examples/test.jpg
is excluded by!**/*.jpg
poetry.lock
is excluded by!**/*.lock
📒 Files selected for processing (9)
README.md
(1 hunks)examples/main.py
(2 hunks)examples/main_async.py
(1 hunks)pyproject.toml
(1 hunks)tests/test_async_client.py
(1 hunks)tests/test_client.py
(3 hunks)upyloadthing/async_client.py
(1 hunks)upyloadthing/base_client.py
(1 hunks)upyloadthing/client.py
(3 hunks)
🔇 Additional comments (19)
upyloadthing/client.py (3)
5-19
: Adopted inheritance from BaseUTApi successfully.Overall, the transition to inherit from
BaseUTApi
appears clean. The docstring updates to reflect the new synchronous client are clear, and no immediate functional issues stand out.
68-115
: Handle multi-file upload partial failures.When uploading a list of files, a single file failure could be masked if the rest succeed. Consider improving error handling so that any partial successes or failures are clearly reported (e.g., storing these results separately or raising a combined exception with detailed info).
117-165
: Ensure alignment of delete/list usage with new key types.The updated methods allow specifying
key_type
and returning typed responses. Ensure that the calling code, especially any existing consumers of these methods, correctly passes and interpretskey_type
. This might require updates elsewhere to avoid mismatched usage.upyloadthing/async_client.py (2)
1-21
: Async client introduction is well-structured.You have added clear type hints and docstrings for the new
AsyncUTApi
. The usage ofasyncio
andhttpx.AsyncClient
is straightforward.
69-128
: Parallel file uploading is nicely implemented.The
_upload_single_file
helper method andasyncio.gather
usage for parallel uploads are a good approach. Just be mindful of concurrency limits such as open file descriptors or remote server constraints.upyloadthing/base_client.py (3)
1-34
: Abstract base class design fosters maintainability.The introduction of
BaseUTApi(ABC)
cleanly separates shared logic from synchronous/asynchronous specifics. This design considerably improves maintainability.
35-64
: Validate token decoding carefully.You decode the base64 token and parse it into
UTtoken
. If the token is malformed or expires, ensure that consumers know how to handle such errors. Allowing more precise exception handling or user-friendly messages might be beneficial.
65-116
: Approve the abstraction for file operations.Defining abstract methods
_request
,upload_files
,delete_files
,list_files
, andget_usage_info
ensures consistent structure across sync and async implementations. The docstrings are comprehensive.examples/main.py (3)
2-2
: LGTM!The
List
import is correctly added to support type hints for the new multi-file upload functionality.
31-33
: LGTM!The implementation demonstrates a clear and well-structured approach to handling multiple file uploads:
- Clear section headers for better readability
- Proper handling of both file system (PNG) and in-memory (JPEG) files
- Correct type hints for the upload results
- Clear result printing
Also applies to: 34-39, 40-43, 44-47, 48-52
54-58
: LGTM!The file deletion logic is correctly updated to handle multiple files:
- Proper extraction of file keys from upload results
- Clear error message printing
examples/main_async.py (4)
1-7
: LGTM!The imports are correctly organized and include all necessary modules for async operations.
9-14
: LGTM!The async main function is properly defined and the client initialization is correct.
15-20
: LGTM!The implementation demonstrates proper async/await usage throughout all API calls:
- Usage info retrieval
- File listing
- Multiple file uploads
- File deletion
Also applies to: 22-30, 32-53, 55-60
63-64
: LGTM!The main function is correctly executed using
asyncio.run()
for the async entry point.tests/test_client.py (1)
138-173
: LGTM!The test for multiple file types is well-structured and verifies:
- Correct handling of different file types
- Proper MIME type detection
- Correct response structure
pyproject.toml (1)
30-30
:✅ Verification successful
LGTM! Addition of pytest-asyncio for async testing.
The addition of pytest-asyncio aligns with the PR objectives to support async operations. The version constraint allows compatible updates.
Let's verify the version compatibility:
🌐 Web query:
What is the latest version of pytest-asyncio and what Python versions does it support?
💡 Result:
The latest version of pytest-asyncio is v0.25.3, released on January 28, 2025[1][4]. This version supports Python 3.8+, with explicit compatibility declarations up to Python 3.13 in recent releases[7][9].
Key details about compatibility:
- Dropped support for Python ≤3.7 starting with v0.23 series (2024)[7]
- Added Python 3.13 support in v0.23 series (2024)[7]
- Requires pytest ≥8 since v0.24 series (2024)[4][7]
Recent compatibility milestones:
Version Python Support Pytest Requirement Notable Changes 0.25.x 3.8+ ≥8 Latest maintenance releases[1][4][7] 0.24.x 3.8+ ≥8 Introduced loop_scope parameter[4][7] 0.23.x ≥3.8 ≥7 Added Python 3.13 support[7][9] The plugin follows semantic versioning and maintains compatibility with actively supported Python versions[1][4][7]. Users should upgrade regularly to receive security patches and new features while checking release notes for any breaking changes[4][7].
Citations:
- 1: https://pypi.org/project/pytest-asyncio/
- 2: https://pypi.org/project/pytest-asyncio/0.12.0/
- 3: https://docs.pytest.org/en/stable/changelog.html
- 4: https://github.com/pytest-dev/pytest-asyncio/releases
- 5: https://packages.debian.org/sid/python3-pytest-asyncio
- 6: https://docs.python.org/3/library/asyncio.html
- 7: https://pytest-asyncio.readthedocs.io/en/latest/reference/changelog.html
- 8: https://tracker.debian.org/python-pytest-asyncio
- 9: https://pytest-asyncio.readthedocs.io
- 10: Support for Pytest 8.0 pytest-dev/pytest-asyncio#737
Verified: pytest-asyncio Dependency Addition Approved
- The inclusion of pytest-asyncio aligns with the PR’s goal to support async testing.
- Web verification confirms that the latest pytest-asyncio release is v0.25.3, which supports Python 3.8+ (with compatibility through Python 3.13) and requires pytest ≥ 8 from v0.24 onward.
- Note that the current version constraint (
^0.23.2
) inpyproject.toml
restricts updates to the 0.23 series (due to pre-1.0 semver rules). This is acceptable if you intend to avoid a pytest upgrade; however, if you want to benefit from newer features or fixes in later releases (v0.24.x or v0.25.x), consider updating the version constraint accordingly.README.md (2)
103-122
: LGTM! Clear documentation of sync and async clients.The documentation clearly distinguishes between UTApi (sync) and AsyncUTApi (async) clients with appropriate examples. This helps users understand the two available patterns.
143-168
: LGTM! Comprehensive documentation of response models.The response models section clearly documents all fields with their types, making it easy for users to understand the API responses.
### Methods | ||
|
||
Both clients provide these methods: | ||
|
||
- `upload_files(files: BinaryIO | List[BinaryIO], content_disposition: str = "inline", acl: str | None = "public-read") -> List[UploadResult]` | ||
- Upload one or more files | ||
- Returns list of upload results | ||
|
||
- `delete_files(keys: str | List[str], key_type: str = "file_key") -> DeleteFileResponse` | ||
- Delete one or more files by key or custom ID | ||
- Returns deletion result | ||
|
||
- `list_files(limit: int | None = None, offset: int | None = None) -> ListFileResponse` | ||
- List uploaded files with optional pagination | ||
- Returns file listing | ||
|
||
- `get_usage_info() -> UsageInfoResponse` | ||
- Get account usage statistics | ||
- Returns usage information |
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.
🛠️ Refactor suggestion
Method signatures are well documented, but error handling needs update.
The method signatures with type hints are clear and helpful. However, there's a discrepancy in the error handling section.
The error handling example uses requests.exceptions.HTTPError
but the client uses httpx
. Update the error handling section to use the correct exception:
-from requests.exceptions import HTTPError
+from httpx import HTTPError
try:
api.upload_files(file)
except HTTPError as e:
Committable suggestion skipped: line range outside the PR's diff.
4bd00bc
to
c564094
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.
Actionable comments posted: 2
🧹 Nitpick comments (11)
examples/main.py (1)
56-60
: Consider adding error handling for file deletion.While the implementation is correct, it would be beneficial to add try-except blocks to handle potential API errors during deletion.
# Delete the uploaded files print("🗑️ Deleting test files...") file_keys = [result.file_key for result in upload_results] - delete_result = api.delete_files(file_keys) - print(f"Deleted {delete_result.deleted_count} file(s)") + try: + delete_result = api.delete_files(file_keys) + print(f"Deleted {delete_result.deleted_count} file(s)") + except Exception as e: + print(f"Error deleting files: {e}")examples/async_main.py (1)
9-63
: Consider using async context manager for API client.For better resource management, consider initializing the API client using an async context manager.
-async def main(): +async def main(): + async with AsyncUTApi() as api: print("🚀 UploadThing API Demo (Async)\n") - - # Initialize the client - api = AsyncUTApi()upyloadthing/client.py (1)
87-90
: Consider adding batch size limit for file uploads.To prevent potential memory issues or API rate limits, consider adding a maximum batch size for file uploads.
+ MAX_BATCH_SIZE = 10 + if len(files) > MAX_BATCH_SIZE: + raise ValueError(f"Maximum batch size of {MAX_BATCH_SIZE} exceeded") + files_data = [ self._prepare_file_data(file, content_disposition, acl) for file in files ]upyloadthing/async_client.py (1)
51-67
: Consider adding retry logic for transient failures.For improved reliability, consider implementing retry logic for transient HTTP errors.
+ async def _request_with_retry(self, *args, max_retries: int = 3, **kwargs): + for attempt in range(max_retries): + try: + return await self._request(*args, **kwargs) + except httpx.TransientHTTPError as e: + if attempt == max_retries - 1: + raise + await asyncio.sleep(2 ** attempt) # Exponential backoffupyloadthing/base_client.py (3)
28-52
: Add validation for region configuration.While the token validation is good, consider adding validation to ensure the provided region exists in
self.token.regions
.self.region = ( options.region if options and options.region else os.getenv("UPLOADTHING_REGION") or self.token.regions[0] ) +if self.region not in self.token.regions: + raise ValueError(f"Region {self.region} not found in available regions: {self.token.regions}") self.api_url = API_URL
144-188
: Enhance file type detection and validation.The current implementation could be more robust in handling file types and validation:
- Consider using python-magic for more accurate file type detection
- Add validation for file size limits
- Validate file name for security
+import magic + def _prepare_file_data( self, file: BinaryIO, content_disposition: str, acl: str | None ) -> dict: file_seed = uuid.uuid4().hex file_key = generate_key(file_seed, self.token.app_id) file_name = getattr(file, "name", f"upload_{uuid.uuid4()}") + # Validate file name + if not self._is_safe_filename(file_name): + raise ValueError("Invalid file name") + file_size = file.seek(0, 2) file.seek(0) - file_type = ( - mimetypes.guess_type(file_name)[0] or "application/octet-stream" - ) + # More accurate file type detection + file_type = magic.from_buffer(file.read(2048), mime=True) + file.seek(0) + + # Validate file size + max_size = 100 * 1024 * 1024 # 100MB example limit + if file_size > max_size: + raise ValueError(f"File size exceeds limit of {max_size} bytes") ingest_url = make_presigned_url( self.region,
189-242
: Enhance error handling with specific error types.Consider creating custom exception types for different error scenarios to allow more granular error handling by clients.
+class UTApiError(Exception): + """Base exception for UploadThing API errors.""" + pass + +class UTApiAuthError(UTApiError): + """Authentication related errors.""" + pass + +class UTApiQuotaError(UTApiError): + """Quota exceeded errors.""" + pass + def _handle_error_response(self, e: httpx.HTTPStatusError) -> None: error_msg = f"UploadThing API error: {e.response.status_code}" try: error_data = e.response.json() if "error" in error_data: error_msg += f" - {error_data['error']}" + # Map status codes to specific exceptions + if e.response.status_code == 401: + raise UTApiAuthError(error_msg) + elif e.response.status_code == 429: + raise UTApiQuotaError(error_msg) except Exception: pass raise httpx.HTTPStatusError( error_msg, request=e.request, response=e.response ) from etests/test_client.py (3)
1-44
: Add edge case test fixtures.Consider adding fixtures for:
- Files with special characters in names
- Empty files
- Files with non-standard MIME types
@pytest.fixture def special_char_file(): """Fixture for file with special characters in name.""" file = BytesIO(b"test content") file.name = "test@#$%.jpg" return file @pytest.fixture def empty_file(): """Fixture for empty file.""" file = BytesIO(b"") file.name = "empty.txt" return file @pytest.fixture def custom_mime_file(): """Fixture for file with custom MIME type.""" file = BytesIO(b"custom content") file.name = "data.custom" return file
46-104
: Enhance test assertions for edge cases.Add assertions to verify:
- Error cases for invalid tokens
- Region validation
- Header content validation
def test_init_with_options(): options = UTApiOptions(token=MOCK_TOKEN, region="eu-west-1") api = UTApi(options) assert api.token.app_id == "test-app" assert api.token.api_key == "test-key" assert api.region == "eu-west-1" + # Add assertions for invalid region + with pytest.raises(ValueError, match="Region .* not found in available regions"): + UTApi(UTApiOptions(token=MOCK_TOKEN, region="invalid-region")) + +def test_init_with_invalid_token(): + """Test initialization with invalid token format.""" + with pytest.raises(ValueError, match="Invalid token format"): + UTApi(UTApiOptions(token="invalid-token"))
105-293
: Add tests for API error responses.Consider adding tests for various API error scenarios:
- Rate limiting responses
- Authentication failures
- Quota exceeded errors
@respx.mock def test_upload_files_rate_limit(respx_mock: respx.MockRouter, ut_api, mock_file): """Test handling of rate limit errors.""" respx_mock.put(url__regex="https://sea2.ingest.uploadthing.com/").mock( return_value=httpx.Response( 429, json={"error": "Rate limit exceeded"} ) ) with pytest.raises(httpx.HTTPError, match="Rate limit exceeded"): ut_api.upload_files(mock_file) @respx.mock def test_upload_files_auth_error(respx_mock: respx.MockRouter, ut_api, mock_file): """Test handling of authentication errors.""" respx_mock.put(url__regex="https://sea2.ingest.uploadthing.com/").mock( return_value=httpx.Response( 401, json={"error": "Invalid API key"} ) ) with pytest.raises(httpx.HTTPError, match="Invalid API key"): ut_api.upload_files(mock_file)🧰 Tools
🪛 Gitleaks (8.21.2)
222-222: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
tests/test_async_client.py (1)
1-292
: Add async-specific test scenarios.Consider adding tests for:
- Concurrent uploads with varying file sizes
- Connection timeouts
- Cancellation handling
@pytest.mark.asyncio @respx.mock async def test_concurrent_uploads_varying_sizes(respx_mock: respx.MockRouter, ut_api): """Test concurrent uploads with varying file sizes.""" files = [ BytesIO(b"small" * 100), # 500B BytesIO(b"medium" * 1000), # 6KB BytesIO(b"large" * 10000), # 60KB ] for i, f in enumerate(files): f.name = f"file{i}.txt" upload_response = { "fileHash": "hash", "url": "https://example.com/file", "ufsUrl": "https://example.com/ufs", "appUrl": "https://example.com/app", } respx_mock.put(url__regex="https://sea2.ingest.uploadthing.com/").mock( return_value=httpx.Response(200, json=upload_response) ) result = await ut_api.upload_files(files) assert len(result) == 3 assert all(isinstance(r, UploadResult) for r in result) @pytest.mark.asyncio @respx.mock async def test_upload_timeout(respx_mock: respx.MockRouter, ut_api, mock_file): """Test handling of connection timeouts.""" respx_mock.put(url__regex="https://sea2.ingest.uploadthing.com/").mock( side_effect=httpx.TimeoutException("Connection timed out") ) with pytest.raises(httpx.TimeoutException): await ut_api.upload_files(mock_file)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
examples/test.jpg
is excluded by!**/*.jpg
poetry.lock
is excluded by!**/*.lock
📒 Files selected for processing (9)
README.md
(1 hunks)examples/async_main.py
(1 hunks)examples/main.py
(2 hunks)pyproject.toml
(1 hunks)tests/test_async_client.py
(1 hunks)tests/test_client.py
(3 hunks)upyloadthing/async_client.py
(1 hunks)upyloadthing/base_client.py
(1 hunks)upyloadthing/client.py
(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- pyproject.toml
🔇 Additional comments (5)
examples/main.py (1)
31-54
: LGTM! The multi-file upload implementation looks good.The code effectively demonstrates how to prepare and upload multiple files simultaneously using the updated API.
upyloadthing/client.py (1)
44-66
: Excellent error handling implementation!The error handling is comprehensive and provides clear error messages. The use of type guards and proper exception chaining is a great practice.
upyloadthing/base_client.py (2)
1-26
: LGTM! Well-organized imports and constants.The imports are logically grouped and the constants are appropriately defined at module level.
65-142
: LGTM! Well-defined abstract interface.The abstract methods are well-documented with comprehensive type hints and clear docstrings.
README.md (1)
88-99
: Update error handling example to use httpx.The error handling example uses
requests.exceptions.HTTPError
but the client useshttpx
.-from requests.exceptions import HTTPError +from httpx import HTTPError try: api.upload_files(file) except HTTPError as e: if e.response.status_code == 413: print("File too large") else: print(f"Upload failed: {e}") except Exception as e: print(f"Unexpected error: {e}")
# Upload both files | ||
upload_results: List[UploadResult] = await api.upload_files( | ||
[png_file, jpeg_file], acl="public-read" | ||
) | ||
|
||
print("Upload results:") | ||
for result in upload_results: | ||
print(f"- {result.name}: {result.file_key}") |
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.
🛠️ Refactor suggestion
Add error handling for concurrent file uploads.
The concurrent upload implementation looks good, but consider adding error handling for individual file failures.
# Upload both files
- upload_results: List[UploadResult] = await api.upload_files(
- [png_file, jpeg_file], acl="public-read"
- )
+ try:
+ upload_results: List[UploadResult] = await api.upload_files(
+ [png_file, jpeg_file], acl="public-read"
+ )
+ except Exception as e:
+ print(f"Error uploading files: {e}")
+ return
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
# Upload both files | |
upload_results: List[UploadResult] = await api.upload_files( | |
[png_file, jpeg_file], acl="public-read" | |
) | |
print("Upload results:") | |
for result in upload_results: | |
print(f"- {result.name}: {result.file_key}") | |
# Upload both files | |
try: | |
upload_results: List[UploadResult] = await api.upload_files( | |
[png_file, jpeg_file], acl="public-read" | |
) | |
except Exception as e: | |
print(f"Error uploading files: {e}") | |
return | |
print("Upload results:") | |
for result in upload_results: | |
print(f"- {result.name}: {result.file_key}") |
# Upload all files in parallel | ||
results = await asyncio.gather( | ||
*[self._upload_single_file(file_data) for file_data in files_data] | ||
) | ||
|
||
return results |
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.
🛠️ Refactor suggestion
Consider adding concurrency control for large batches.
While using asyncio.gather
for concurrent uploads is efficient, consider adding a semaphore to limit concurrent connections for large batches.
+ # Limit concurrent connections
+ semaphore = asyncio.Semaphore(5) # Limit to 5 concurrent uploads
+
+ async def upload_with_semaphore(file_data):
+ async with semaphore:
+ return await self._upload_single_file(file_data)
+
# Upload all files in parallel
results = await asyncio.gather(
- *[self._upload_single_file(file_data) for file_data in files_data]
+ *[upload_with_semaphore(file_data) for file_data in files_data]
)
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
# Upload all files in parallel | |
results = await asyncio.gather( | |
*[self._upload_single_file(file_data) for file_data in files_data] | |
) | |
return results | |
# Limit concurrent connections | |
semaphore = asyncio.Semaphore(5) # Limit to 5 concurrent uploads | |
async def upload_with_semaphore(file_data): | |
async with semaphore: | |
return await self._upload_single_file(file_data) | |
# Upload all files in parallel | |
results = await asyncio.gather( | |
*[upload_with_semaphore(file_data) for file_data in files_data] | |
) | |
return results |
Introduces async API client and refactors codebase for dual sync/async support
Adds a new AsyncUTApi class for asynchronous operations while refactoring the existing UTApi into a synchronous implementation. Both clients inherit from a new BaseUTApi abstract class that defines common functionality and interface methods.
Changes
Impact
Summary by CodeRabbit
Documentation
New Features
Tests
Chores
Refactor