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

Maven - Add support for no-proxy & https proxy #1925

Merged
merged 5 commits into from May 18, 2023
Merged

Maven - Add support for no-proxy & https proxy #1925

merged 5 commits into from May 18, 2023

Conversation

Or-Geva
Copy link
Contributor

@Or-Geva Or-Geva commented May 1, 2023 •
edited

  • All tests passed. If this feature is not already covered by the tests, I added new tests.
  • All static analysis checks passed.
  • This pull request is on the dev branch.
  • I used gofmt for formatting the code before submitting the pull request.

Part of:

@Or-Geva Or-Geva added the new feature Automatically generated release notes label May 1, 2023
@Or-Geva Or-Geva requested a review from yahavi May 1, 2023 08:30
This was referenced May 1, 2023
Merged
Merged
yahavi
yahavi requested changes May 9, 2023
View reviewed changes
maven_test.go
setEnvCallBack := clientTestUtils.SetEnvWithCallbackAndAssert(t, utils.HttpProxyEnvKey, "http://login:pass@proxy.mydomain:8888")
defer setEnvCallBack()
// Set noProxy to match all to skip http proxy configuration
setNoProxyEnvCallBack := clientTestUtils.SetEnvWithCallbackAndAssert(t, utils.NoProxyEnvKey, "*")
Copy link
Member

Choose a reason for hiding this comment

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

Would it work for "proxy.mydomain"? If so, let's use it instead of *

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, "proxy.mydomain" is the actual proxy host. no-proxy refers to the server that should avoid proxy redirects.

maven_test.go
setHttpsEnvCallBack := clientTestUtils.SetEnvWithCallbackAndAssert(t, utils.HttpsProxyEnvKey, "https://logins:passw@proxys.mydomains:8889")
defer setHttpsEnvCallBack()
// Set noProxy to match all to skip https proxy configuration
setNoProxyEnvCallBack := clientTestUtils.SetEnvWithCallbackAndAssert(t, utils.NoProxyEnvKey, "*")
Copy link
Member

Choose a reason for hiding this comment

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

Can we change here a concatenation of multiple no_host?
For example:
NO_PROXY="*.aventail.com,home.com,.seanet.com,proxys.mydomains"

@Or-Geva Or-Geva added the safe to test Approve running integration tests on a pull request label May 18, 2023
@github-actions github-actions bot removed the safe to test Approve running integration tests on a pull request label May 18, 2023
@Or-Geva Or-Geva added the safe to test Approve running integration tests on a pull request label May 18, 2023
@github-actions github-actions bot removed the safe to test Approve running integration tests on a pull request label May 18, 2023
@Or-Geva Or-Geva merged commit a1bd810 into jfrog:dev May 18, 2023
57 of 59 checks passed
@chenrui333 chenrui333 mentioned this pull request May 23, 2023
Merged
@Or-Geva Or-Geva mentioned this pull request Jul 10, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yahavi yahavi yahavi requested changes

Assignees
No one assigned
Labels
new feature Automatically generated release notes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@Or-Geva @yahavi