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

Remove deflate option, switch to gzip #21

Merged
merged 4 commits into from
Aug 20, 2014
Merged

Conversation

jfirebaugh
Copy link
Contributor

No description provided.

if (solid === false) return callback(err, buffer, headers);
// Empty tiles are equivalent to no tile.
if (source._blank || !key) return callback(new Error('Tile does not exist'));
// Fake a hex code by md5ing the key.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@yhahn The comment here was a lie -- it md5'd the buffer, not the key, and thus returned different solid values depending on if it was deflated or not. I switched it to do what the comment says and updated the test, but I don't know what the consequences of this are.

Copy link
Member

Choose a reason for hiding this comment

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

thx for the heads up -- dug up this change back here:

3400f33

which I did a horrible job documenting.

@yhahn
Copy link
Member

yhahn commented Aug 16, 2014

Sounds good -- after thinking through this I reached a similar conclusion (that deflate/gzip should be done in putTile by the module doing the actual storage I/O).

The corresponding PRs here would be a gzip option in tilelive-s3 and node-mbtiles?

We're going with gzip as defacto now and handling both deflate/gzip encodings in consuming modules.

@yhahn yhahn changed the title Remove deflate option Remove deflate option, switch to gzip Aug 20, 2014
jfirebaugh added a commit that referenced this pull request Aug 20, 2014
Remove deflate option, switch to gzip
@jfirebaugh jfirebaugh merged commit 31b6105 into master Aug 20, 2014
@jfirebaugh jfirebaugh deleted the content-encoding branch August 20, 2014 18:37
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