search

twitchtv / twirp

A simple RPC framework with protobuf service definitions
https://twitchtv.github.io/twirp/docs/intro.html
Apache License 2.0
7.13k stars 327 forks source link

support gzipped response #230

Closed knqyf263 closed 4 years ago

knqyf263 commented 4 years ago

When a response size is large, it is better to compress it. I've already implemented it in https://github.com/twitchtv/twirp/pull/225, but @dpolansky said it should be opt-in. Let me discuss how we should implement it. As for a client, we may be able to use twirp.ClientOption without breaking backward compatibility.

dpolansky commented 4 years ago

Have you considered implementing server-side response compression by wrapping the HTTP handler's ResponseWriter? Keep in mind that there are some nuances with wrapping ResponseWriter depending on which interfaces your program expects the handler's ResponseWriter to implement.

In general, it would be ideal for users to be able to implement compression without requiring changes to Twirp itself, based on the principle of keeping Twirp's spec simple and its API surface area small.

knqyf263 commented 4 years ago

I think compression is useful for many users. It would be great if Twirp itself implements it, so I sent PR, but if it is not the case, I can implement it on my side and you can close this issue.

dpolansky commented 4 years ago

I don't think the utility of the feature for some users is in question. I think the consequences of supporting this feature in Twirp's protocol are significant, so it's important to consider whether alternative solutions (such as the one I described) are sufficient. I'm closing this issue for now, but discussion on the topic is welcome in general.