Skip to content

Migrate to AsyncStreaming from swift-async-algorithms#160

Merged
FranzBusch merged 12 commits into
mainfrom
fb-async-stream-migraiton
May 15, 2026
Merged

Migrate to AsyncStreaming from swift-async-algorithms#160
FranzBusch merged 12 commits into
mainfrom
fb-async-stream-migraiton

Conversation

@FranzBusch
Copy link
Copy Markdown
Member

Motivation

The AsyncStreaming module here was only a temporary for us to get the API proposals up and running. We knew that this is not the final destination. I moved them over to swift-async-algorithms this week.

Modifications

This PR migrates all towards the new AsyncStreaming module that is based on RangeReplaceableContainer. Since this touches everything in this repo the changes are quite large. I broke them down into individual commits that can be reviewed in isolation.

A few important things to call out:

  1. The concluding variants of the reader and writes moved to the HTTPAPIs module. I intend to follow up on them and make them HTTP specific.
  2. There are a few helpers for the writer and reader that also moved to the HTTPAPIs module. I intend to move them over to swift-async-algorithms in a subsequent PR.

Result

We are now using the new AsyncStreaming module from swift-async-algorithms

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Intention is to move this over to swift-async-algorithms in a subsequent PR.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Intention is to move this over to swift-async-algorithms in a subsequent PR.

@FranzBusch FranzBusch added the 🆕 semver/minor Adds new public API. label May 14, 2026
@_exported public import HTTPAPIs

#if canImport(Darwin) || os(Linux)
public import BasicContainers
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Will BasicContainers and UniqueArray move to the stdlib at some point? Would this prevent putting the API in the SDK?

Copy link
Copy Markdown
Member Author

@FranzBusch FranzBusch May 14, 2026

Choose a reason for hiding this comment

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

Yes they will. My hope is that InputSpan/OutputSpan will conform to RangeReplaceableContainer themselves and would allow us to not have a concrete type at all for this. Otherwise we will most likely want to use the default bag of bytes type that the stdlib folks decide. Could be either Data or UniqueData or something else.

) async throws(AsyncStreaming.EitherError<any Error, Failure>) -> Return where Failure: Error {
let data: Data?
do {
data = try await self.actual.data(maximumCount: nil)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Will there be a read with a limit in the future? The code was complex and tricky to get right, so would be great if we can get rid of it completely

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The reason that I removed the read limit is that it becomes essentially impossible to implement with containers unless containers are self-slicing.

Copy link
Copy Markdown
Collaborator

@guoye-zhang guoye-zhang left a comment

Choose a reason for hiding this comment

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

The change looks fine, though it is unfortunate that we are incurring an extra copy on both the reading side and the writing side. Let's land it first.

@FranzBusch FranzBusch merged commit c195a5d into main May 15, 2026
15 of 16 checks passed
@FranzBusch FranzBusch deleted the fb-async-stream-migraiton branch May 15, 2026 10:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🆕 semver/minor Adds new public API.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants