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

Do not upload files with identical etags #536

Closed
wants to merge 1 commit into from

Conversation

denisdefreyne
Copy link
Member

This is continued off #480.

This is quite tricky to write tests for. Not sure how to handle that.

@denisdefreyne
Copy link
Member Author

CC @nanoc/contributors — would like a +1

@mpapis
Copy link
Member

mpapis commented Mar 7, 2015

yes looks good 👍, I guess my only remark is that the run grows quite big, but it's not related to this PR

@denisdefreyne
Copy link
Member Author

Will look into adding some tests for this. (Will be hard, I guess!)

@denisdefreyne denisdefreyne modified the milestone: 3.7.6 Apr 19, 2015
local_etag = calc_local_etag(file_path)
remote_etag = etags[key]
next if remote_etag && remote_etag == local_etag

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

avoid calculating local hash if remote is not available:

remote_etag = etags[key]
if remote_etag
  local_etag = calc_local_etag(file_path)
  next if remote_etag == local_etag
end

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, updated! (see #552—not this PR)

@mpapis
Copy link
Member

mpapis commented Apr 19, 2015

to simplify testing you could extract https://github.com/nanoc/nanoc/blob/aws-deploy-md5-v2/lib/nanoc/extra/deployers/fog.rb#L68-L86 to separate method upload - this also solves problem of growing run - and add attr_accessor :directory, :etags, :keys_to_destroy - this way you can check your custom directory and list of etags without actually connecting to aws.

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

Successfully merging this pull request may close these issues.

None yet

2 participants