-
Notifications
You must be signed in to change notification settings - Fork 13
Streaming Fix - JSDK-298 #579
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
|
It doesn't look like we've enabled git lfs for this repo. We probably should do that and push that zip file in there. |
| size = channel.read(dst); | ||
| position += size; | ||
| return size; | ||
| return Long.valueOf(size).intValue(); |
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.
Did the lack of this cause problems when a blob was over 4GB?
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.
This is one of the places it failed, I can't say if this is wholly the issue though.
| .map(bulkObject -> bulkObject.getOffset()) | ||
| .max(Long::compareTo).orElseGet(() -> blob.getOffset()); | ||
|
|
||
| final boolean isReadOnly = ((SeekableByteChannelDecorator) seekableByteChannel).wrappedSeekableByteChannel() instanceof ReadOnlySeekableByteChannel; |
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.
Why is the cast here needed in order to perform the instanceof check?
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.
We know it is a SeekableByteChannelDecorator because we wrap it further up, but the wrappedSeekableByteChannel() is only on the Decorator
| return makePutTransferMethod(); | ||
| } | ||
| }); | ||
| (client, masterObjectList, eventDispatcher) -> new PutSequentialBlobStrategy(ds3Client, |
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.
👍
* Fixed * Oops * Imports * Imports * Imports
* JSDK-297: Add new Header value to HEAD Object. (#577) * JSDK-297: Add new Header value to HEAD Object. Adding in parsing for new headers 'creation-date' and 'version-id' into the head object response payload. * JSDK-297: Add new Header value to HEAD Object. Generated head object call with new header parsing. * Streaming Fix - JSDK-298 (#579) * Fixed * Oops * Imports * Imports * Imports * cherrypicking streaming blob change and reving to 5.0.6 * Merging missing changes from 5.1.2 Co-authored-by: RachelTucker <RachelTucker@users.noreply.github.com> Co-authored-by: Eric <192110+scribe@users.noreply.github.com>
This is a bit of a hack, but it passes all the tests now.