S3 downloads are not interruptable
Describe the bug
For instance, hitting Ctrl-C during
nix eval --impure --expr 'builtins.fetchurl "s3://highlighter-public/405b3ba02ccb2f07ec6aba9f233d4194.mp4?region=ap-southeast-2"'
does nothing.
Steps To Reproduce
Expected behavior
Nix should exit with error: interrupted by the user quickly.
Probably the easiest way to do this is to have S3Helper::getObject() use an std::stringstream wrapper that calls checkInterrupt().
nix-env --version output
Additional context
Add any other context about the problem here.
Priorities
Add :+1: to issues you find important.
@edolstra ill work on this issue if you can assign me this
As a first-time contributor, I'd like some help. Where should I implement the new InterruptibleStringStream class (like in libutils or a specific folder)? Also, is this 👇🏽 where is the S3Helper is implemented ? https://github.com/NixOS/nix/blob/dbcd4cd6bab50ffbad795feec404758644415815/src/libstore/s3-binary-cache-store.cc#L92
I'm somewhat confused by the current implementation. Why does S3 fetching live inside curlFileTransfer, while having absolutely nothing to do with curl. The code is already littered with FIXME/TODO and the "easy" fix would pile even more complexity on this class. SRP is totally broken here. Maybe the better thing to do would be to define an abstract interface for fetching and move the code related to S3 into its own implementation?
Also S3 fetching is already problematic and it's not clear how it handles compressed streams? Does the AWS SDK handle the decompression (can it even do that?) or should it be handled the same way that it's done for regular HTTP with libarchive and TarArchive and Sinks/Sources? https://github.com/NixOS/nix/issues/11261
It would be very helpful to define interface boundaries so that there are clear and defined requirements and api for file transfers/stream decompression/progress tracking. This would be better in the long run than piling amazingly complex logic into a single class.
I've made an attempt in #12227, it's a little delayed at closing out but it does seem to improve the problem.
Fixed by #13752