Conversation
nazywam
left a comment
There was a problem hiding this comment.
Few possible issues raised internally, other than that LGTM
|
@psrok1, @nazywam Hello! I wonder why you decided to add support of async in karton. What measurable profit did you found with asyncs? For me, I see only disadvantages:
For the second case, it could be better to make sync code as a wrapper of async code, but I am not an expert in these case :) |
|
@rakovskij-stanislav motivation is in the feature documentation: https://karton-core.readthedocs.io/en/latest/asyncio.html.
It's a good strategy if you write async-first code that additionally supports sync operations, but Karton was not that case. The code is less of a problem than the interface: it's easier to use conventions made for async code in sync code than the other way. |
Here I'm making a PoC for asyncio support in Karton. Changes improving code reusability between sync/async parts will be isolated into separate PRs.
Changes in core:
self.current_taskfrom closure will see None, but I hope nobody does such thing.karton.core.asyncio.backend.KartonAsyncBackendwith asyncio reimplementation of backend methods needed by Producers and Consumers. Non-I/O parts of code that are common forKartonBackendandKartonAsyncBackendwere moved into KartonBackendBase.KartonAsyncBackendworks a bit differently thanKartonBackend:await KartonAsyncBackend.connect()method.async with self.s3 as clientto get the client object)KartonBind.is_asyncattribute to indicate that consumer is asyncio-based, which may be visualized in karton-dashboard.Task.unserializeacceptsresource_unserializerResource factory as an argument to move the creation of RemoteResource objects to the KartonBackend.backendargument is marked as deprecated.asyncio.RemoteResource.contentraises RuntimeError when resource was not explicitly downloaded into memory before by calling adownload()method.KartonLogHandler.set_taskmethod, current task is always got fromtask.get_current_taskthat is using a ContextVar. GeneralizedKartonBase.log_handlertype to generic logging.Handler. Actually we may consider supporting any log_handler as aKartonBaseargument in case someone wants to customize the logging (out of scope for this PR)