axman6 / amazonka-s3-streaming

Provides a conduit based interface to uploading data to S3 using the Multipart API
MIT License
20 stars 23 forks source link

Amazonka-2.0 conversion #28

Closed axman6 closed 2 years ago

axman6 commented 2 years ago

Ping @endgame, @jhrcek

Currently comparing with the other branch to see just the 2.0 differences.

domenkozar commented 2 years ago

I'm happy to test once this PR is merged! Thanks for all the hard work.

domenkozar commented 2 years ago

Note that this is the last blocker for https://cachix.org to migrate to GHC 9.2 / Stackage nightly

axman6 commented 2 years ago

Hey @domenkozar, thanks for pointing that out, I had no idea. I’ll see if I can get to it today.

axman6 commented 2 years ago

Ping @domenkozar, @koterpillar, @mpickering, @jhrcek - after a lot of help from @endgame I'm finally merging this with full amazonka-2.0 compatibility (including a fix for a longstanding amazonka bug for HashedStreams of file parts). 🎉

domenkozar commented 2 years ago

Awesome work, I'll test this as soon as I can!

axman6 commented 2 years ago

Thanks @domenkozar - I've merged this into master and it'll be what we use going forward. Let me know if you run into any issues, particularly related to cachix!

jhrcek commented 2 years ago

@axman6 thanks for pushing this through! I'll update our app and test this version and get back if I find any issues. I take it that you included all the changes from https://github.com/axman6/amazonka-s3-streaming/ so you're probably not going to publish that PR as separate library version on hackage (not that I want it, I'm fine with 2.0 compatible version)?

axman6 commented 2 years ago

@jhrcek I’m sure sure if you linked to the right thing, but any changes that were in the rewrite-play branch are in this branch, in one form or another. This is basically a complete rewrite of the conduit code, but the concurrent upload is roughly the same - the diff is probably a bit too large to see what’s changed easily, however. I don’t really have any plans to maintain an Amazonka 1.X version once 2.0 is released, because people really should migrate to the new style once it’s available.

jhrcek commented 2 years ago

Sorry, wanted to link this PR https://github.com/axman6/amazonka-s3-streaming/pull/25 If that's the case you should probably just close that PR

endgame commented 2 years ago

I wouldn't bother maintaining anything supporting 1.6.x - it's so far in the past and hashedFileRange is fundamentally broken, which means amazonka-s3-streaming is at least partially unfixably broken (for the file upload stuff, I suppose).

axman6 commented 2 years ago

Yeah I’ll do some clean up of issues and PRs once things have hit hackage.

domenkozar commented 1 year ago

Note that I keep using my fork because the current requires the upload location to be a strict ByteString, which means I'd need to load gigabytes of data first into memory before streaming it to amazonka - which defeats the purpose of this library :)

axman6 commented 1 year ago

Not sure, I understand, the library allows streaming upload of bytestring data, or concurrent upload of a large strict bytestring or file on disk - what's the use case you want that isn't covered?

domenkozar commented 1 year ago

Imagine that the source is already a streamed lazy bytestring that shouldn't consume more than a few MB of memory coming from the network.

endgame commented 1 year ago

Can you turn your lazy ByteString into a Monad m => ConduitT i ByteString m () and feed it to streamUpload instead of concurrentUpload?