Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Support for S3::Object#stream #80

Merged
merged 1 commit into from
Oct 29, 2013
Merged

Support for S3::Object#stream #80

merged 1 commit into from
Oct 29, 2013

Conversation

dlecocq
Copy link

@dlecocq dlecocq commented Oct 29, 2013

Aimed at #35

qoobaa added a commit that referenced this pull request Oct 29, 2013
Support for S3::Object#stream
@qoobaa qoobaa merged commit 7659fd6 into qoobaa:master Oct 29, 2013
@qoobaa
Copy link
Owner

qoobaa commented Oct 29, 2013

Thanks.

@dlecocq
Copy link
Author

dlecocq commented Oct 29, 2013

Sorry, there appears to be a problem with using this in practice. Apparently the body is getting loaded implicitly somewhere, and the second invocation (with stream) is throwing an exception. I'm trying to track it down at which point I'll update the tests and PR

@qoobaa
Copy link
Owner

qoobaa commented Oct 30, 2013

Please create a PR if you fix it :-). Maybe I'll find some time to debug it soon.

@dlecocq
Copy link
Author

dlecocq commented Oct 30, 2013

It all comes down to Net::HTTP insisting on reading the body when a request is made. In particular request /does/ allow a block during which the body has not been read. But after the block is invoked, it reads whatever content in the body hasn't been consumed by the block.

I have a fix working, but it makes things a fair amount uglier. Perhaps there's a better adapter to use?

I'm mostly interested this because I'm constrained to using JRuby, and have found other libraries for S3 access to either not play nicely with JRuby or to make some odd assumptions. I'll push a new branch once I've got tests passing, though they need a bit of rejiggering.

@dlecocq
Copy link
Author

dlecocq commented Oct 30, 2013

Added a PR: #81

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

Successfully merging this pull request may close these issues.

2 participants