-
Notifications
You must be signed in to change notification settings - Fork 17.8k
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: remove SizedReaderAt #15818
Comments
Sorry, I strongly disagree here. Interface io.SizedReaderAt was not intended for os.File, but for any use case in which the size is known ahead of time. If anything this could be made clearer in the docs. Correct usage of io.SizedReaderAt with an os.File would be to wrap it in io.SectionReader with the results of a previous os.Stat-call. Then the ReadAt-Method will correctly report an error. I could prepare a CL with a better example here. NOTE: Undoing a days long discussion for a years long feature request in just 3h is a bit fast, isn't it? |
New API can be added at any point in time. But once an API has been part of a release, it cannot be removed again. Since the next release is in feature freeze and past the point for long winded discussions, it is better to remove a questionable API again instead of being stuck with it. It can be re-evaluated in the next iteration. |
To respond to the NOTE, what @dominikh is exactly right. We need to discuss this further, it would be best to do that when we're not under the crunch of getting a release out the door (that's a good way to make bad decisions), and in any event the people needed for the discussion (at the least, me and Brad) will not be available simultaneously again until September (Brad is away now and I will be away when he returns). There's basically no harm to delaying this one cycle. Thanks for filing the new issue. |
CL 21492 added this interface during the Go 1.7 cycle. The CL description says:
(A later CL renamed it to SizedReaderAt.)
I tried in CL 23374 to make os.File implement this interface, since it already implements io.ReaderAt. But the natural Size method there needs to return an error: the underlying call to find the size may fail, and that failure needs to be reported. Returning 0 is incorrect, since the caller might conclude the file is empty, returning -1 may break various slice bounds preparing for a read, and returning a very large number could result in very large allocations. In this case, there needs to be some way to report an actual error.
This seems like a general problem: ReadAt can return an error, indicating that the underlying resource can go bad, but Size cannot. There's no obvious reason why Size fundamentally can't fail. Certainly if the file were accessed over the network, trying to find the Size could easily fail. Contrast this with ReadSeeker, which is often used for non-concurrent random access to data: the Seek(0, 2) operation reports the size of the data, but also an error.
This problem with the inability to report errors suggests that, contrary to the CL description, the SizedReaderAt interface may actually not be best practice.
I'm going to remove it for Go 1.7:
it's late in the Go 1.7 cycle, there are questions about the semantics, Brad is away, and anyone who really needs this interface can continue to define it themselves for another cycle. (Also the name is still awkward.)
The text was updated successfully, but these errors were encountered: