feat(async transport): Add Async Transport implementation #4654
+1,382
−95
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
…ted a sync transport HTTP subclass Moved shared sync/async logic into a new superclass (HttpTransportCore), and moved sync transport specific code into a new subclass(BaseSyncHttpTransport), from which the current transport implementations inherit Fixes GH-4568
Removed an unnecessary TODO message and reverted a class name change for BaseHTTPTransport. GH-4568
Adds test coverage for the error handling path when HTTP requests return error status codes. GH-4568
Restore comments accidentally removed during a previous commit.
Refactored class names such that BaseHttpTransport now has the same functionality as before the hierarchy refactor GH-4568
Add a new flush_async method in the Transport ABC. This is needed for the async transport, as calling it from the client while preserving execution order in close will require flush to be a coroutine, not a function. GH-4568
Move flush_async down to the specific async transport subclass. This makes more sense anyway, as this will only be required by the async transport. If more async transports are expected, another shared superclass can be created. GH-4568
Add necessary type annotations to the core HttpTransport to accomodate for async transport. GH-4568
Add an abstract bass class for implementation of the background worker. This was done to provide a shared interface for the current implementation of a threaded worker in the sync context as well as the upcoming async task-based worker implementation. GH-4578
Add a new factory method instead of direct instatiation of the threaded background worker. This allows for easy extension to other types of workers, such as the upcoming task-based async worker. GH-4578
Add a new flush_async method to worker ABC. This is necessary because the async transport cannot use a synchronous blocking flush. GH-4578
Move the flush_async down to the concrete subclass to not break existing testing. This makes sense, as this will only really be needed by the async worker anyway and therefore is not shared logic. GH-4578
Coroutines have a return value, however the current function signature for the worker methods does not accomodate for this. Therefore, this signature was changed. GH-4578
Add a new implementation of the worker interface, implementing the worker as an async task. This is to be used by the upcoming async transport. GH-4581
Refactor the flush method in the async worker to use the async_flush coroutine. GH-4581
…unctions Add a check to see wheter callbacks are awaitable coroutines or functions, as coroutines need to be awaited. GH-4581
…coroutines Coroutines do not return None, therefore it is necessary to consider this in the callback parameter of the worker. Previously, only callbacks with return Type None were accepted. GH-4581
Enable concurrent callbacks on async task worker by firing them as a task rather than awaiting them. A done callback handles the necessary queue and exception logic. GH-4581
Changed kill to also use the _TERMINATOR sentinel, so the queue is still drained to this point on kill instead of cancelled immediately. This should also fix potential race conditions with flush_async. GH-4581
Add proper type annotation to worker task list to fix linting problems GH-4581
Add an implementation of Transport to work with the async background worker and HTTPCore async. GH-4582
Async Transport now properly checks for the presence of the event loop in capture_envelop, and drops items in case the event loop is no longer running for some reason. GH-4582
Implement a kill method that properly shuts down the async transport. The httpcore async connection pool needs to be explicitly shutdown at the end of its usage. GH-4582
Fix type errors resulting from async override and missing type definition in the async transport. GH-4582
Add a try/catch to ensure silent fail on kill in case the event loop shuts down. GH-4582
Fix the event loop check in make_transport so that it does not throw a runtime error but rather falls back correctly. GH-4582
The callbacks passed to the worker from the transport are all async now, so this is currently not needed. GH-4581
Add an implementation of Transport to work with the async background worker and HTTPCore async. GH-4582
Async Transport now properly checks for the presence of the event loop in capture_envelop, and drops items in case the event loop is no longer running for some reason. GH-4582
Implement a kill method that properly shuts down the async transport. The httpcore async connection pool needs to be explicitly shutdown at the end of its usage. GH-4582
Fix type errors resulting from async override and missing type definition in the async transport. GH-4582
Add a try/catch to ensure silent fail on kill in case the event loop shuts down. GH-4582
Fix the event loop check in make_transport so that it does not throw a runtime error but rather falls back correctly. GH-4582
Make kill optionally return a task for async transport. This allows for a blocking kill operation if the caller is in an async context. GH-4582
❌ 1310 Tests Failed:
View the top 3 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
antonpirker
changed the title
Previously, the Async Transport fallback pointed to the sync superclass, which does not implment core methods. This now falls back to the correct, implemented sync transport GH-4601
AsyncIO integration is necessary for the Async Transport to work properly. This adds a check that makes AsyncTransport fall back to Sync Transport if this is not configured. GH-4601
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.
Contains the complete implementation of the async transport, with potel-base merged into it again.
The async transport was initially implemented in the following PR's:
Transport Class Abstraction
Worker Class Abstraction
Async Task Worker
Async Transport Implementation
Async Transport SDK Integration