-
Notifications
You must be signed in to change notification settings - Fork 92
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
Add HTTP Proxy support to NPM Binary Installer #1067
Add HTTP Proxy support to NPM Binary Installer #1067
Conversation
@farawaysouthwest: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Apollo Contributor License Agreement here: https://contribute.apollographql.com/ |
👷 Deploy request for apollo-cli-docs pending review.Visit the deploys page to approve it
|
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.
Thanks for the PR
LGTM.
thanks for your help @kumarrishav! I have not dealt with no_proxy before and was not aware of that requirement. |
yeah. Np. Thank you for doing this |
…y-npm-installer" This reverts commit f4f8277.
@farawaysouthwest would you mind filing a follow up PR to use the TypeScript package you linked? |
unfortunately, this doesn't seem working because of the possible bug in Axios while using HTTPS over HTTP proxy some resource: |
Let me raise the PR to handle these scenario |
Yes, in this case it will be https_proxy for GitHub
But these things depends on organization.
For ex:
What if if a organization has added GitHub in no_proxy. I.e don't use proxy
for GitHub.com
As per current implementation, it will force the proxy and connection might
fail.
Hence, suggested to have generic solution.
Thanks,
Kumar Rishav
…On Wed, Apr 6, 2022, 5:34 PM Mitchell Alderson ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In installers/npm/binary.js
<#1067 (comment)>:
> @@ -136,6 +140,30 @@ const install = () => {
}
};
+const configureProxy = () => {
+ // get proxy env
+ const env = process.env.HTTP_PROXY || process.env.HTTPS_PROXY
I see what you mean, but I think due to this line:
https://github.com/farawaysouthwest/rover/blob/5f57ddd553ad581f1f551a85c124668a87d0b100/installers/npm/binary.js#L95
... the access url would always be HTTPS?
—
Reply to this email directly, view it on GitHub
<#1067 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AA7DV5PDV4TP4YNJQB7NWADVDYGPRANCNFSM5SW33LOQ>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
Proposed fix for: #899
Adds a simple method to create an axios compliant proxy object from the HTTP/HTTPS_PROXY env's, and pass in as part of
fetchOptions
for the install function. (https://github.com/EverlastingBugstopper/binary-install/blob/main/packages/binary-install/index.js#L54)