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

add support for gzip response (issue #409) #481

Closed
wants to merge 1 commit into from
Closed

add support for gzip response (issue #409) #481

wants to merge 1 commit into from

Conversation

tyiss
Copy link

@tyiss tyiss commented Aug 17, 2015

Adds the gzip directive to nginx.conf.
this will enable the gzip module, http://nginx.org/en/docs/http/ngx_http_gzip_module.html.

@sonicaghi
Copy link
Member

can you pls write a detailed description on what this PR does and how? Also if you can have a round of test passing.

thx!

@tyiss
Copy link
Author

tyiss commented Aug 18, 2015

Sure,

the tests were failing on my end.
I'll fix them later today (hopfully) and add the descripsion in #489.
בתאריך 18 באוג׳ 2015 13:11,‏ "Augusto Marietti" notifications@github.com
כתב:

can you pls write a detailed description on what this PR does and how?
Also if you can have a round of test passing.

thx!


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

@montanaflynn
Copy link

This PR will only gzip text/html which is not likely to be used in most APIs. There are many options available for the built in nginx gzip directive that should be reasoned about rather than just simply turning it "on" or "off" at the highest context level.

I would also think this should be optional as either a plugin or through kong.yml up at the top (much like the shared dict size) since for many use cases gzipping all HTTP responses is not suitable.

@ahmadnassri
Copy link
Contributor

as @montanaflynn pointed out, this is better done as a plugin, rather than blanketed across all proxied requests.

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.

4 participants