Skip to content
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

Only support tuple[int, int] for ByteRangeRequests #2567

Closed
maxrjones opened this issue Dec 16, 2024 · 5 comments
Closed

Only support tuple[int, int] for ByteRangeRequests #2567

maxrjones opened this issue Dec 16, 2024 · 5 comments

Comments

@maxrjones
Copy link
Member

maxrjones commented Dec 16, 2024

I don't think there should be two different ways to specify "the beginning of the buffer".

this also holds for zarr-python -- I think we currently use None and 0 to express the same thing, which should be fixed.

Originally posted by @d-v-b in #1661 (comment)

Based on this discussion, None should not be supported as the start or end for ByteRangeRequests.

@maxrjones
Copy link
Member Author

I would be glad to work on this if helpful.

@d-v-b
Copy link
Contributor

d-v-b commented Dec 16, 2024

thanks for opening this issue! I think always using a non-negative int for the start of a byte range will work, but I'm not sure about the end -- I suspect None might be necessary to model an unknown size. Someone who knows the stores better than I do (cc @normanrz ) would have a better idea here.

@maxrjones
Copy link
Member Author

I think always using a non-negative int for the start of a byte range will work, but I'm not sure about the end -- I suspect None might be necessary to model an unknown size.

Ah, this actually brings up a third scenario which might also motivate typing of tuple[int, int | None]. The ObjectStore PR supports a suffix request with the length indicated by a negative start value. Local store seems to support this as well, with an additional possibility of a negative end value. So in both cases negative values indicate the number of bytes from the end of the file. FsspecStore does not seem to support this.

I was really surprised to discover ByteRangeRequest is actually [start, length] rather than [start, end] given the typical structure of range headers. I think this will need to be very prominently documented for potential extensions (e.g., as far as I can tell #1661 was written assuming ByteRangeRequest represents [start, end]).

@d-v-b
Copy link
Contributor

d-v-b commented Dec 16, 2024

We should minimize surprises here, so I would support normalizing our byte range convention to match what people expect

@maxrjones
Copy link
Member Author

Darn, I'm sorry that I missed that this is a duplicate of #2437. Let's continue any discussion there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants