-
Notifications
You must be signed in to change notification settings - Fork 104
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
Brotli support? #45
Comments
@galaczi I'm glad you like this project, I'm also using it almost everywhere :) I've never used brotli yet, gzip was enough for my use case, but I see it could be useful. I think it shouldn't be a very hard task to add it to this project. I would greatly appreciate your help with adding brotli support, PRs are welcomed! |
We'd very much welcome Brotli support with this image. |
@Valian @galaczi So we've been doing some digging into this. The nginx brotli module needs to be compiled on build, so it needs to be done a few image levels up. So, the Dockerfile would need to be modified to something like this:
|
nginx.brotli.conf would look like this:
|
Effectively, we're hoping to be publishing an image that this repository can user for Brotli support here: asencis/docker-openresty-nginx-brotli:latest So we can modify the Dockerfile.brotli for this project to:
|
Thanks @asencis for all the work you've done here! It seems like a very nice way forward to add Brotli support to this image. Do you think you could work on a PR? I'm a bit busy recently, but if you won't be able to push it forward I'll try to do it by myself. |
@Valian Hi Valian - this image is now live and we're deploying it in production, without issue. HTTPS/SSL from you side of things is all excellent and working, and our content encoding is indeed set to br when available., gzip when not. (And also ZStandard when available as we have some middleware on our application layer). What would the PR consist of? The previous image is essentially your Dockerfile, but I could PR a much larger one? |
This is a great solution. I will be using it all over the place.
Any chance of adding brotli support?
The text was updated successfully, but these errors were encountered: