[Proposal] SOAR-0009: Type-safe streaming multipart support

Hi folks!

The proposal SOAR-0009: Type-safe streaming multipart support for Swift OpenAPI Generator is now up and In Review:

The review period will run until November 15 - please feel free to post your feedback either as a reply to this thread, or on the pull request.

Thanks!

cc @Honza_Dvorsky

3 Likes

Great proposal! Nothing to add really apart from a naming suggesting in the PR.
Looking forward to actually using this!

1 Like

Quickly went through the proposal, looks really good so far. I suppose, under the hood, there is going to be a brand new multipart runtime implementation for the OpenAPI tools and you won't utilize third-party libs, such as multipart-kit, right?

1 Like

Correct, because we need a fully streaming representation of the multipart body, and the individual parts. However, as you point out, it's an implementation detail that can be swapped out later if MultipartKit gets similar streaming support. The API is pretty minimal, so I think we could do that in an API-nonbreaking way.

Proper streaming support in MultipartKit is on the roadmap

2 Likes

Great to hear. I'd love to be able to eventually remove that code from the runtime library and use the SSWG-incubated variant in MultipartKit. The custom implementation here is there right now for two reasons, 1. streaming and 2. SwiftNIO dependency. If streaming can be added and SwiftNIO removed, it could in theory become a dependency of the OpenAPI runtime library in the future.

The use of exports is going to continue to bite us for a while, however we might be able to manage it. We use two types from NIO. HTTPHeaders which I think we can switch out with the HTTP types library. The other type we use is ByteBuffer (and friends). How does the proposed code handle streaming data, i.e. what is the type returned, Data? The lack of a Swift 'bag of bytes' type keeping popping up across the ecosystem as is a blocker to us dropping SwiftNIO in a number of projects

1 Like

why not do the “obvious” thing here and lower ByteBuffer into its own module within the swift-nio repository?

Two reasons - partly because I have no control over SwiftNIO :sweat_smile: and partly because you're still pulling in the whole of NIO which is probably not desirable.

Having a new package swift-bytebuffer that is then a dependency on NIO and re-exported to keep the API is a possible solution but I've no idea how much work that is and if it's even feasible

to be clear, i am not advocating for splitting the repositories further - it is already far too balkanized and spinning off a separate swift-bytebuffer repo is neither necessary nor beneficial.

i think we should reexamine the assumption here:

in my view, SPM cloning all the artifacts from swift-nio is not a major issue right now.

sure, in general, the way SPM fetches dependencies has some major performance problems. but these problems are not specific to swift-nio, and it makes no sense to micro-optimize away a swift-nio fetch in 5.9, when every project that uses macros must clone swift-syntax in order to compile.

if swift-syntax didn’t exist, sure it might make sense to evaluate tradeoffs between cloning swift-nio and cloning a smaller, separate repo. but swift-syntax does exist, it isn’t going away any time soon, and i think it is okay if the fly that is swift-nio piggybacks on the elephant that is swift-syntax.

Right, there are general ecosystem improvements that'd help here, but Swift OpenAPI Runtime will not be taking a dependency on SwiftNIO.

As you point out, @0xTim, we use swift-http-types's HTTPFields as our header fields type.

The bag of bytes we use is ArraySlice<UInt8> and I'm happy enough with it. We also have a HTTPBody type that is an async sequence of ArraySlice<UInt8>, which is our currency type for streaming HTTP body bytes.

What I think is important here is that we're designing the API to allow the multipart implementation to eventually be swapped out, once multipart-kit has what we need. But until then we'll land multipart support with our custom parser/serializer, to allow us to meet our goal of getting to 1.0 next month.

If you have any thoughts on the multipart proposal, I'd love to hear them :slightly_smiling_face:

2 Likes

The review period for this proposal has now concluded. As Honza summarised, there were some good comments about how this could build on future ecosystem-wide work, if/when that is available, but the general feedback on the feature proposal, given the dependencies we can use today, was positive.

Thanks to all those who provided feedback, here and on the PR.

1 Like