Skip to content
This repository has been archived by the owner on Jul 2, 2020. It is now read-only.

Http request / resonse max size parameter #73

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Http request / resonse max size parameter #73

wants to merge 2 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Apr 5, 2018

Some of our services produce rather big responses, bigger than the default of 5 megabytes. So I added two additional settings which are used in the serviceFactory to create the http clients.

@codecov-io
Copy link

codecov-io commented Apr 5, 2018

Codecov Report

Merging #73 into master will increase coverage by 0.23%.
The diff coverage is 71.42%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #73      +/-   ##
==========================================
+ Coverage   35.04%   35.27%   +0.23%     
==========================================
  Files          29       29              
  Lines         799      805       +6     
  Branches       34       34              
==========================================
+ Hits          280      284       +4     
- Misses        519      521       +2
Impacted Files Coverage Δ
.../com/twitter/diffy/proxy/HttpDifferenceProxy.scala 46.15% <0%> (-3.85%) ⬇️
...n/scala/com/twitter/diffy/DiffyServiceModule.scala 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7894459...2f11c3d. Read the comment docs.

@@ -73,6 +75,12 @@ object DiffyServiceModule extends TwitterModule {
var httpsPort =
flag[String]("httpsPort", "443", "Port to be used when using HTTPS as a protocol")

var maxRequestSize =
Copy link
Contributor

Choose a reason for hiding this comment

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

These requirements are independent of SSL and are generally useful. Please use these settings in a shared trait between Http and Https. Bonus points for adding this support at a lower finagle layer so that it may be directly configured in DifferenceProxy to support Thirft in addition to http and https.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

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.

3 participants