Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add support of proxy environment variable (#191) #199

Merged
merged 2 commits into from
Jun 2, 2020
Merged

Add support of proxy environment variable (#191) #199

merged 2 commits into from
Jun 2, 2020

Conversation

francois-roget
Copy link
Contributor

Allow the CLI to take into account http_proxy or https_proxy environment varaibles to perform the network requests. This is particularly handy in corporate CI.

This pull request makes the following changes:

  • Add a getHttpAgent method in RequestHelpers to get a configured http.Agent (or undefined)
  • Use this http.Agent for all fetch calls.
  • Test the http.Agent generation in RequestHelpers.spec

It relates to the following issue #s:

cc @bhamail / @DarthHater / @allenhsieh / @ken-duck

@sonatypecla
Copy link

sonatypecla bot commented May 29, 2020

Thanks for the contribution! Before we can merge this, we need @francois-roget to sign the Sonatype Contributor License Agreement.

DarthHater
DarthHater reviewed May 29, 2020
View reviewed changes
src/Services/RequestHelpers.ts Outdated
@@ -14,6 +14,8 @@
* limitations under the License.
*/
import os from 'os';
import { Agent } from 'http';
const HttpsProxyAgent = require('https-proxy-agent');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm, can this not be imported like import {HttpsProxyAgent} from 'https-proxy-agent';?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DarthHater the code has been changed as requested.
I made a fixup. If this version is agreed, I'll squash it to have a single commit

@DarthHater
Copy link
Member

@francois-roget one minor question, if you wouldn't mind taking a look!

@sonatypecla sonatypecla bot removed the cla:signed label Jun 2, 2020
@sonatypecla
Copy link

sonatypecla bot commented Jun 2, 2020

Thanks for the contribution! Unfortunately we can't verify if the committer(s), François Roget francois.roget@ingenico.com, signed the CLA because they have not associated their commits with their GitHub user. Please follow these instructions to associate your commits with your GitHub user. Then sign the Sonatype Contributor License Agreement and this Pull Request will be revalidated.

@francois-roget
Copy link
Contributor Author

@DarthHater I corrected according to your remark.

ButterB0wl
ButterB0wl approved these changes Jun 2, 2020
View reviewed changes
Copy link
Contributor

@ButterB0wl ButterB0wl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good to me! thanks for doing this I knew it was one of those little things that would have to be addressed but couldn't get to

README.md
* If you are not a Sonatype customer, Do NOT file Sonatype support tickets related to nancy support in regard to this project, file an issue here on GitHub
- If you are a Sonatype customer, you may file Sonatype support tickets related to `AuditJS` support in regard to this project
- We suggest you file issues here on GitHub as well, so that the community can pitch in
- If you are not a Sonatype customer, Do NOT file Sonatype support tickets related to nancy support in regard to this project, file an issue here on GitHub
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DarthHater bit of a typo here :P

src/Services/RequestHelpers.spec.ts Show resolved Hide resolved
@DarthHater DarthHater merged commit 276974d into sonatype-nexus-community:master Jun 2, 2020
DarthHater pushed a commit that referenced this pull request Jun 2, 2020
chore(release): 4.0.17 [skip ci]
eba2625 
## [4.0.17](v4.0.16...v4.0.17) (2020-06-02)

### Bug Fixes

* Add support of proxy environment variable ([#191](#191)) ([#199](#199)) ([276974d](276974d))
@DarthHater
Copy link
Member

🎉 This PR is included in version 4.0.17 🎉

The release is available on:

Your semantic-release bot 📦🚀

@snyk-bot snyk-bot mentioned this pull request Jan 22, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DarthHater DarthHater DarthHater approved these changes

@ButterB0wl ButterB0wl ButterB0wl approved these changes

Assignees
No one assigned
Labels
cla:signed released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[FEATURE] Add support of corporate proxy
3 participants
@francois-roget @DarthHater @ButterB0wl