Skip to content

Proof of concept for async pipe - #252

Open
edmundmills wants to merge 1 commit into
dbrattli:mainfrom
edmundmills:async-pipe
Open

Proof of concept for async pipe#252
edmundmills wants to merge 1 commit into
dbrattli:mainfrom
edmundmills:async-pipe

Conversation

@edmundmills

Copy link
Copy Markdown

This PR creates an async_pipe that works with coroutines. It can be passed either synchronous or asynchronous functions as arguments, and calls them sequentially on the input, awaiting the coroutines appropriately.

The async_pipe uses inspect.iscoroutine to determine whether results should be awaited. An earlier version used inspect.isawaitable, but this did not work with the Result world, since Results are awaitable. The same would be true for Options.

It also includes an extra bind function for async_result that can be used in the async_pipeline.

I would personally find this functionality helpful for integrating with existing code that uses the standard python approach to async.

@dbrattli dbrattli left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the proof of concept. Before it can be reconsidered, please rebase onto current main and address these blockers:

  1. extra.async_result.bind accepts a mapper returning Result[_TResult, Any] but promises Result[_TResult, _TError]. That is type-unsound: a mapper may introduce a different error type undetected. Preserve _TError in the mapper return type and use Awaitable rather than the overly specific Coroutine.
  2. async_pipe recursively calls itself once per pipeline stage, so a sufficiently long valid runtime pipeline raises RecursionError. Implement the execution loop iteratively while retaining the deliberate iscoroutine behavior that avoids awaiting Option and Result.

Please add tests for error-type preservation and a pipeline longer than Python’s recursion limit; then CI can validate the rebased implementation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants