client: Handle get_blob futures being polled serially

[cgwalters]

No description provided.

[Copilot]

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

[allisonkarlitskaya]

Thanks for turning this into a PR!

This is fine by me, but I wonder if it violates the original purpose of the check that you mentioned in #71 (ie: we don't want to allow the client to successfully read to EOF if we have a checksum error about to be reported)...

[allisonkarlitskaya]

Thanks for the docs here, btw.

[cgwalters]

(ie: we don't want to allow the client to successfully read to EOF if we have a checksum error about to be reported)...

Yeah, it's a fair point! At least today you'd have to explicitly drop the driver future on the floor to run into this problem. If you don't drop the driver future but don't ? on it then you'll get a warning.

I think the most elegant thing here would actually be to return a new struct that just impl BufRead but also has a finish() method that's required to be called?

A related option is to make the reader only start returning data once the driver has been polled once - that would pretty strictly enforce the use of join!.

[allisonkarlitskaya]

...starts to sound like the linear types discussion all over again.

I honestly think the best case scenario here might be to try to plumb the checksum failure through the reader API. You objected to that last time we talked about it, I think, but it would completely avoid the need for a separate driver object...

It's possible to attach arbitrary additional errors objects to stdlib results...

[cgwalters]

I wouldn't object at all support to moving the checking into the reader, it's not unusual to wedge things like this into std::io::ErrorKind::Other I guess.

Though there's another option of offering a GetBlobUnverified that allows the caller to do checksumming instead.

[allisonkarlitskaya]

composefs-rs does the checksum of the uncompressed data only, which is the only thing that's important to us... In fact, I think we completely discard the manifest at present.

[cgwalters]

OK for now I changed this to just document things.

[allisonkarlitskaya]

Hopefully I/we don't forget about this one again ;)