-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
cli: use proxy settings when downloading plugins #7747
Conversation
@slhultgren can you tell me if this fixes your issue? It should support all |
bb36391
to
5bd5c11
Compare
@marechal-p Thanks for including the lastError change as well. I tested it and it works well for me. It properly respects the url and uses HTTP_PROXY for http urls and HTTPS_PROXY for https urls. Also ALL_PROXY works as well. |
5bd5c11
to
fd0f617
Compare
@vince-fugnitto @marcdumais-work just checked the new deps with ClearlyDefined, everything looks good to me. |
a7c9135
to
8e7ac88
Compare
lastError is always set to undefined at the end of each for-loop iteration, this is not intended behavior. The lastError should persist when exiting the loop -if set-. This commit also simplifies stream piping and error handling. Signed-off-by: Paul Maréchal <paul.marechal@ericsson.com>
request/requestretry used to support some set of proxy environment variables, while node-fetch doesn't by default. This commit add support for proxy settings when downloading plugins. Signed-off-by: Paul Maréchal <paul.marechal@ericsson.com>
8e7ac88
to
6786e1f
Compare
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 verified that the previous behavior (without proxy) still works correctly 👍
For the proxy use-case it's been confirmed to work offline as well.
request/requestretry used to support some set of proxy environment
variables, while node-fetch doesn't by default.
This commit add support for proxy settings when downloading plugins.
Fixes #7739
How to test
Review checklist
Reminder for reviewers