REST API: Add compression to REST API #1390
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #1390 +/- ##
===========================================
- Coverage 65.37% 65.36% -0.02%
===========================================
Files 79 79
Lines 11279 11283 +4
===========================================
+ Hits 7374 7375 +1
- Misses 3340 3342 +2
- Partials 565 566 +1
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
id-ms
force-pushed
the
enable-gzip-in-rest
branch
from
5a2a7c1 to
2503d34
Compare
id-ms
changed the title
api/server.go
Outdated
| // we currently support compressed result only for GET /v2/blocks/ API | ||
| Skipper: func(c echo.Context) bool { | ||
| return !strings.Contains(c.Path(), "/v2/blocks/") || c.QueryParam("compress") != "true" | ||
| }, |
There was a problem hiding this comment.
| // we currently support compressed result only for GET /v2/blocks/ API | |
| Skipper: func(c echo.Context) bool { | |
| return !strings.Contains(c.Path(), "/v2/blocks/") || c.QueryParam("compress") != "true" | |
| }, |
The gzip middleware looks like it can write any response in gzip, so while I know the exact issue we're trying to solve is for the blocks endpoint, I don't see a reason not to support it for all APIs.
Also, I think the proper way to request compression from the client side is the Accept-Encoding header. The echo middleware also supports this so it looks like we don't need to do anything further in terms of query params parsing here.
For example, the go-algorand-sdk should be able to get gzip'd output via
client.LookupBlock(rnd).Do(ctx, &common.Header{Key: "Accept-Encoding", Value: "gzip"})
There was a problem hiding this comment.
I limited the ability to use gzip just for the /v2/blocks endpoint because I think we need to add tests for all endpoints. In order to avoid that and just address the problem we have, I think it is a good start. If you still think we should support all endpoints I can do that, but adding tests for all endpoints might take a while.
As for your second point, do you suggest we use gzip every time the client support gzip encoding? even if it didn't request it? My approach was only to add the support and not replace it (don't want to break any 3rd party code)
There was a problem hiding this comment.
That's fair, let's limit it to the blocks endpoint for now then.
Existing code probably isn't supplying the Accept-Encoding: gzip header. Without specifying the header explicitly, the middleware will won't compress anything, so I expect almost all clients to have the same behavior by default.
| waitForServer(t, listenAddr) | ||
|
|
||
| getBlockFunc := func(t *testing.T, headerOnly bool, useCompression bool) *generated.BlockResponse { | ||
| path := "/v2/blocks/1" |
There was a problem hiding this comment.
If we make this a parameter, we can generalize this across a bunch of different APIs and test to make sure compression works in general and not just for blocks.
There was a problem hiding this comment.
Since we're just testing the blocks endpoint for now, we can pull this into a separate function for other APIs later.
Summary
In order to more easily support returning responses for blocks containing large numbers of transactions, we're adding gzip compression as an available response content encoding.
In order to have a response gzip'd (limited in this PR only to the
/v2/block/endpoint) clients can provide the header:For example, in the go-algorand-sdk, you could use the following code to retrieve a compressed block:
See https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Accept-Encoding for more details on the
Accept-Encodingheader.Test Plan
Added tests for compressed responses from /v2/blocks.