Add transport worker factory and abstract base class for background worker #4580
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
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
Codecov ReportAttention: Patch coverage is
✅ All tests successful. No failed tests found.
Additional details and impacted files@@ Coverage Diff @@
## srothh/transport-class-hierarchy #4580 +/- ##
====================================================================
- Coverage 84.90% 84.90% -0.01%
====================================================================
Files 158 144 -14
Lines 15373 15044 -329
Branches 2423 2377 -46
====================================================================
- Hits 13053 12773 -280
+ Misses 1573 1543 -30
+ Partials 747 728 -19
|
The uncovered lines of code here are just the pass calls from the abstract class definition. |
…ass-hierarchy ref(transport): Add flush_async to Transport ABC
Add a new flush_async method to worker ABC. This is necessary because the async transport cannot use a synchronous blocking flush. GH-4578
…ass-hierarchy Merge the class structure changes from transport
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
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.
Add a new abstract base class for the background worker implementations in the transport. This allows for implementation of the upcoming async task-based worker in addition to the current thread based worker used in the sync context. Additionally, add a factory method in the HttpTransportCore shared class, to allow the worker methods to live higher up in the class hierarchy regardless of specific implementation.
GH-4578