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

Fix bug so that sync verify works with gzip correctly #107

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

giratikanon
Copy link

Currently, if you try to sync files with verify: true and gzip: true, it compares the md5 of the gzipped remote file with the non-gzipped local file, so the hashes never match.

So if the local file is newer, sync will always upload, regardless of whether the content changed. For example, when a cron runs every minute and writes out a data file, it will always be uploaded to S3 even if the content is the same as the remote file.

This fixes the problem by just gzipping the local file before getting the md5 hash, assuming gzip: true. It could be more efficient by caching the gzipped file and using it for upload later. In practice, even uploading 100+ files, it still takes just a second or two, the same as before.

Before, sync verify would compare md5 of gzipped remote file and non-gzipped local file, ensuring they would never match.
@zdexter
Copy link
Collaborator

zdexter commented Dec 11, 2013

Nice catch. Could you add some tests that confirm that verify+gzip behavior is fixed, and make sure this passes CI?

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.

2 participants