Skip to content
This repository has been archived by the owner on Jan 15, 2022. It is now read-only.

Add support to request for compressed payloads from the HRavenRestClient #161

Merged
merged 3 commits into from
Apr 10, 2017

Conversation

piyushnarang
Copy link

Noticed that even with counter filtering (#160), some of our payloads are multiple MB in size. The hraven server seems to support gzip as an accept-encoding option and when we use that it compresses the payload substantially in some cases (e.g. one call when down to 32K from 1.45MB). Can observe this behavior via curl:

curl -v --output /dev/null  -H "Accept-Encoding: gzip" -X GET  "http://hraven.devel.host/api/v1/tasks/procrev@atla/job_1486504431708_30155?include=taskType&include=taskId"

> GET /api/v1/tasks/procrev@atla/job_1486504431708_30155?include=taskType&include=taskId HTTP/1.1
> Host: hraven.devel.host
> User-Agent: curl/7.51.0
> Accept: */*
> Accept-Encoding: gzip
> 
  0     0    0     0    0     0      0      0 --:--:--  0:00:07 --:--:--     0< HTTP/1.1 200 OK
< Content-Type: application/json
< Server: Jetty(6.1.26)
< Content-Encoding: gzip
< Content-Length: 32677

I updated the UrlDataLoader to use the apache HTTP libraries. The libraries are setup to use Accept-encoding = "gzip,deflate" by default. In case users of HRavenRestClient explicitly want to not use compression, they can opt out.
Tested the code out with compression on and off.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.003%) to 2.148% when pulling bf21fae on piyushnarang:client-gzip into e17ec79 on twitter:master.

LOG.info("Not using compression!");
httpClientBuilder.disableContentCompression();
} else {
LOG.info("Using compression by default! Trying gzip, deflate");
Copy link

Choose a reason for hiding this comment

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

let's move it to debug.

Copy link
Author

Choose a reason for hiding this comment

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

sure

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, chatted with Piyush offline. We can change the default one using compression to debug mode and keep the "not using compression" in info mode

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.03%) to 2.124% when pulling 111167f on piyushnarang:client-gzip into e17ec79 on twitter:master.

@vrushalivc vrushalivc merged commit 419699d into twitter:master Apr 10, 2017
@vrushalivc
Copy link
Contributor

Thanks @piyushnarang

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants