-
Notifications
You must be signed in to change notification settings - Fork 9
Option to respect range during cache streaming #169
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Requesting a change for at least the comment formatting, but also to open a discussion about the API in general.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple of minor things that should be addressed before merge.
Failing CI |
92e5c23
to
6071bea
Compare
The core cache API allows for cache items to be concurrently streamed to / from the cache. If: - The size of the cached item's body was not provided by the writer - The reader requests a specific range of the cached item's body (`to_stream_from_range`) - The writer and reader are concurrent, i.e. the body is streamed from one to the other then today, the core cache API will ignore the requested range and provide the whole body. There is no explicit notification that the whole body is provided instead of a range. In this SDK release, this remains the default behavior. However, the lookup options struct now now offers an `AlwaysUseRequestedRange` option. If set to true, body reads conducted as part of this lookup/transaction will always use the requested range, even when streaming. If false, the behavior is unchanged (as above). In a future SDK release, the default behavior will change to "always use requested range".
Co-authored-by: Cameron Walters (cee-dub) <[email protected]>
Co-authored-by: Cameron Walters (cee-dub) <[email protected]>
6071bea
to
ebe1351
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Can you either squash these commits on the PR or have github do it when you merge?
Done as a squash on the PR, with an updated message reflecting the behavior. |
The core cache API allows for cache items to be concurrently streamed to / from the cache.
Previously, if:
then the core cache API will ignore the requested range and provide the whole body. There is no explicit notification that the whole body is provided instead of a range.
In this SDK release we change the default: body reads through the cache will use the requested range even when streaming. A lookup option allows setting the legacy behavior.