-
-
Notifications
You must be signed in to change notification settings - Fork 4.8k
Use http agents for hook requests #4791
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
Conversation
Codecov Report
@@ Coverage Diff @@
## master #4791 +/- ##
==========================================
- Coverage 92.69% 92.66% -0.03%
==========================================
Files 119 119
Lines 8679 8686 +7
==========================================
+ Hits 8045 8049 +4
- Misses 634 637 +3
Continue to review full report at Codecov.
|
Do you think this is mergable as is? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah let’s go, not a big deal
@@ -131,6 +131,8 @@ export interface ParseServerOptions { | |||
startLiveQueryServer: ?boolean; | |||
/* Live query server configuration options (will start the liveQuery server) */ | |||
liveQueryServerOptions: ?LiveQueryServerOptions; | |||
/* Keep hook HTTP connections alive */ | |||
hookKeepAlive: ?boolean; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps we should default to true? What would be the drawback?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should flip the default (and potentially make it not configurable) in the future. Just wanted to dip our toe in here.
There should be very little drawback to enabling this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
leave trailing cr
@TylerBrock I believe you forgot to run buildDefinitions to regenerate the parser and options mapper for the CLI :/ |
I'm so sorry, I was totally unaware we had to do that portion of it. Is that documented somewhere? |
No worry! My bad not catching it. To rebuild: |
We've found that hooks create an enormous amount of http traffic (including dns lookups, socket churning, and port exhaustion).
Using a NodeJS http(s) agent with keepAlive enabled in both the client sdk and parse-server has been valuable for us as we scale our production load so I'm proposing this change upstream as it might help others (and could potentially be the default when people feel comfortable).