pnxtech / hydra

A light-weight library for building distributed applications such as microservices
https://www.hydramicroservice.com
MIT License
645 stars 54 forks source link

[WIP] replace "request" package #89

Closed ecwyne closed 7 years ago

ecwyne commented 7 years ago

Currently the request package is used in two fairly simple places. (_tryAPIRequest and _makeAPIRequest)

My initial testing has this PR working for me, though I am not at all sure that it will work in all circumstances. This PR serves to raise the question about rewriting how http requests are performed in hydra.

metrics taken with cost-of-modules

Before

before

After

after

Creating a fake request function is not what I would want to push to production, but it seemed to me like rewriting _makeAPIRequest and _tryAPIRequest is above my pay grade at this point. I believe that using the native http library or something like node-fetch would be a better solution long term

ecwyne commented 7 years ago

Also probably worth looking into setting redis and redis-url as peer dependencys in the fwsp-redis-connection package (as they're being loaded twice)

cjus commented 7 years ago

@ecwyne thanks for this callout and the intro to cost-of-modules. I used node-fetch early on and found some issues with sending binary - non-JSON payloads and also with calling external API - Twillio I believe. So I dropped it. That said, any issue I encountered could have been fixed a while ago. I'll have another look.

cjus commented 7 years ago

Currently considering https://github.com/tomas/needle

ecwyne commented 7 years ago

@cjus thanks for the shoutout on twitter!

I looked into needle at first too, but it looked like it would require rewriting _tryAPIRequest and _makeAPIRequest so I didn't pursue them any further myself.

cjus commented 7 years ago

@ecwyne thanks for the heads up on this. I made the changes because I closely understand how request was being used. I also had to test across the services we're using.

The new hydra:1.2.0 is considerably smaller. About 70%.

cursor_and_1__ec2-user_ip-172-31-6-97____bash_
ecwyne commented 7 years ago

@cjus looks awesome! That's SUBSTANTIALLY smaller, well done!