Add proxy support via global-agent #442
Conversation
src/cache.js
Outdated
| @@ -2,6 +2,9 @@ import got from "got"; | |||
| import logger from "./logger.js"; | |||
| import { parseSchema } from "./parser.js"; | |||
|
|
|||
| import { bootstrap } from "global-agent"; | |||
| bootstrap(); | |||
There was a problem hiding this comment.
Can you explain the thought process behind initialising this in cache.js?
I'd have thought loading it right at the top of the app bootstrap process (index.js) would make the most sense.
There was a problem hiding this comment.
It's the only place got (and therefore global-agent) is used. Wanted to keep them at the same place.
But as mentioned beforehand: my knowledge with node is limited. So feel free to move the code anywhere else.
There was a problem hiding this comment.
If it works in index.js I think I'd be inclined to put it there. Although cache.js is the only places that got is invoked at the moment that might not always be true. If we can bootstrap this right at the "top" of the app, then its less likely I will break proxy functionality as the codebase evolves.
This is especially important given this is a feature that I am not going to be using myself. In fact, its a feature that is quite hard for me to test in any way. I don't have access to a proxy server (I guess I could install one locally). I looked over a bunch of projects on GitHub that use global-agent hoping to see how to write a test for this and I can't see any with an automated test for the proxy functionality.
There was a problem hiding this comment.
Sounds reasonable! Thanks for your feedback!
I just moved the code to index.js in 75d0696
muffl0n
force-pushed
the
add-proxy-support
branch
from
6cdf5c9
to
75d0696
Compare
|
New dependencies detected. Learn more about Socket for GitHub ↗︎
|
Fixes #438