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

Try using @tempfile.size for TempObject#size #318

Merged
merged 3 commits into from Jul 14, 2014

Conversation

rykov
Copy link
Contributor

@rykov rykov commented Feb 7, 2014

If @tempfile is available, TempObject should be checking @tempfile.size instead of doing equivalent of File.size(@tempfile.path). In my particular case, @tempfile is actually a delegate to a remote (S3) object that separately (and lazily) pulls metadata (like size) and the file contents. This change would prevent the file contents to be fetched to disk to just compute the file size.

@markevans
Copy link
Owner

thanks - though I see the Travis build is failing as a number of sizes in tests (ruby 1.8.7) are reporting size as zero (do you know why this is?) https://travis-ci.org/markevans/dragonfly/builds/18384163

@rykov
Copy link
Contributor Author

rykov commented Feb 7, 2014

Yup, I saw that -- I will take a look.

@markevans
Copy link
Owner

any update on this? if not I'll close if you don't mind

@rykov
Copy link
Contributor Author

rykov commented Jul 2, 2014

Yup, sorry - had some trouble installing 1.8.7 - looking at it now.

@rykov
Copy link
Contributor Author

rykov commented Jul 2, 2014

I found the issue. In 1.8.7, a Tempfile object will report a size of 0 after it's been closed - even if it has data and hasn't been unlinked yet. This problem is fixed in 1.9+, so I've added a core_ext to address it in 1.8.7

@markevans
Copy link
Owner

cool thanks. is this a well-known bug in 1.8.7? just asking coz in general I'm reluctant to introduce core extensions unless really necessary.
having said that, I can see that the extra line introduced in that if statement isn't going to harm anything

@rykov
Copy link
Contributor Author

rykov commented Jul 14, 2014

Reaching EOL in a couple of weeks, I don't think this bug will be fixed in
1.8.7.

https://www.ruby-lang.org/en/news/2014/07/01/eol-for-1-8-7-and-1-9-2/

@markevans that said, how long do you think you'll support 1.8.7 for this
Gem?

On Monday, July 14, 2014, Mark Evans notifications@github.com wrote:

cool thanks. is this a well-known bug in 1.8.7? just asking coz in general
I'm reluctant to introduce core extensions unless really necessary.
having said that, I can see that the extra line introduced in that if
statement isn't going to harm anything


Reply to this email directly or view it on GitHub
#318 (comment).

@markevans
Copy link
Owner

fair enough. good point - not forever (especially now that 1.8.7 support is ending)
I'll merge this in

markevans pushed a commit that referenced this pull request Jul 14, 2014
Try using @tempfile.size for TempObject#size
@markevans markevans merged commit 561e212 into markevans:master Jul 14, 2014
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

Successfully merging this pull request may close these issues.

None yet

2 participants