-
Notifications
You must be signed in to change notification settings - Fork 18k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
io: Add MultiReadSeeker #7263
Comments
As described at http://golang.org/doc/contribute.html it works best if you discuss this sort of thing on the mailing list golang-dev@googlegroups.com rather than in the issue tracker. I don't personally have a strong feeling about this. It doesn't *have* to live in the standard library--you could turn this into a separate go-gettable package. But of course we do have io.MultiReader. But then io.MultiReader seems to have more clear uses. But the App Engine use case is compelling. See what people think on the mailing list. Labels changed: added repo-main, release-none. |
There's an implementation of MultiReaderAt in the slides for http://talks.golang.org/2013/oscon-dl.slide#52 which runs dl.google.com: http://talks.golang.org/2013/oscon-dl/sizereaderat.go I've used it several other times. I'd also like to see io.SizedReaderAt: a block device, essentially. It knows its Size() int64 and can ReadAt. |
The only problem I see with MultiReaderAt is that the standard io.ReaderAt doesn't have any means for determining the size of the reader so a new interface has to be created. This is not the case of ReadSeeker where the size can be determined using Seek. Otherwise all the types from the standard library implementing ReadSeeker also implement ReaderAt (although on some platforms like Windows it's emulated by Seek and Read). |
What I was referring to was that MultiReadSeeker will work with the go types as they are without any change. The SizeReaderAt will require to: 1. Add the SizeReaderAt interface to io 2. Make sure that all the types with ReaderAt have the Size method. It should probably rather be LenReaderAt because bytes.Reader and strings.Reader have it already. However, it's missing in os.File and would have to be added. Of course (2) could be skipped but then it requires that the user has to implement it and that's something which doesn't feel "right" - the thing I love about go is how things can be combined out of the box. This doesn't mean I don't find it a good idea - I think it would be useful to have some standard interface defining the name of the length-returning method. Lenner, yuck. |
CL https://golang.org/cl/21492 mentions this issue. |
ReadAtSizer is a common abstraction for a stateless, concurrently-readable fixed number of bytes. This interface has existed in various codebases for over 3 years (previously usually named SizeReaderAt). It is used inside Google in dl.google.com (mentioned in https://talks.golang.org/2013/oscon-dl.slide) and other packages. It is used in Camlistore, in Juju, in the Google API Go client, in github.com/nightlyone/views, and 33 other pages of Github search results. It is implemented by io.SectionReader, bytes.Reader, strings.Reader, etc. Time to finally promote this interface to the standard library and give it a standard name, blessing it as best practice. Updates #7263 Updates #14889 Change-Id: Id28c0cafa7d2d37e8887c54708b5daf1b11c83ea Reviewed-on: https://go-review.googlesource.com/21492 Reviewed-by: Rob Pike <r@golang.org>
I'm not sure I've read this issue before. MultiReadSeeker seems too specialized. In any event, punting to Go 1.11 but maybe we should just close it. |
@rsc I believe it would be useful in the cases that you need to read from the same underlying Readers in the MultiReader. Right know you need to work with each *os.File (if thats your Seeker) individually. I see some implementations in the wild: https://github.com/search?q=MultiReadSeeker&type=Code&utf8=%E2%9C%93 (or rather: a lot of copy-paste implementations of MultiReadSeeker) Not sure if it warrants a implementation in stdlib though. |
I think it's better to have a way to logically concatenate ReaderAt values (using e.g. my https://godoc.org/go4.org/readerutil#NewMultiReaderAt) and then if you want Seek and Read, you wrap it in an SectionReader (https://golang.org/pkg/io/#NewSectionReader). I'm going to close this for now. All these interfaces deserve some thought for Go 2, so I'll flag this issue as Go 2 to make sure it's even easier to find back. |
The text was updated successfully, but these errors were encountered: