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.

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

Set jvm proxy from client environment variables #713

Merged
merged 3 commits into from
Nov 28, 2018

Conversation

crakjie
Copy link

@crakjie crakjie commented Nov 13, 2018

The test detectsMacrosInClasspath wich call directly ch.epfl.scala currently not work under a proxy.
Except that all other frontend/test is working.

I had initially called the proxy setting function in the run function but this function is bypass by the tests which by the way need proxy too.

Copy link
Contributor

@jvican jvican left a comment

Choose a reason for hiding this comment

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

Hey @crakjie, I've left a few comments regarding the policy of modifying environment variables globally. Aside from this, it looks good 👍 Could we also add a detailed explanation in the FAQ (https://scalacenter.github.io/bloop/faq/) of our website so that people know how our proxy support works? I would explain users the following two possibilities to use their proxys with bloop:

  1. Run the server with environment variables (if they can).
  2. Otherwise, tell how the mechanism to pick up environment variables from clients work via Nailgun.

project/BuildPlugin.scala Outdated Show resolved Hide resolved
frontend/src/main/scala/bloop/cli/Proxy.scala Outdated Show resolved Hide resolved
frontend/src/main/scala/bloop/cli/Proxy.scala Outdated Show resolved Hide resolved
frontend/src/main/scala/bloop/cli/Proxy.scala Outdated Show resolved Hide resolved
frontend/src/main/scala/bloop/cli/Proxy.scala Outdated Show resolved Hide resolved
frontend/src/main/scala/bloop/cli/Proxy.scala Outdated Show resolved Hide resolved
frontend/src/main/scala/bloop/cli/Proxy.scala Outdated Show resolved Hide resolved
frontend/src/main/scala/bloop/cli/Proxy.scala Outdated Show resolved Hide resolved
frontend/src/main/scala/bloop/cli/Proxy.scala Outdated Show resolved Hide resolved
frontend/src/main/scala/bloop/cli/Proxy.scala Outdated Show resolved Hide resolved
@jvican jvican added bug A defect or misbehaviour. docs ergonomics Any change that affects developer ergonomics and the easiness of use of bloop. community Any change or proposal that is contributed by the Open Source Community. labels Nov 14, 2018
@jvican jvican changed the title Set jvm proxy #698 Set jvm proxy from client environment variables Nov 15, 2018
@jvican
Copy link
Contributor

jvican commented Nov 19, 2018

@crakjie I'm waiting for your changes here 😄 Whenever you have them ready, push them (make sure you rebase on latest master too) and I merge them.

@crakjie crakjie force-pushed the set-jvm-proxy#698 branch 2 times, most recently from f950c0b to aafa51b Compare November 26, 2018 09:23
@crakjie
Copy link
Author

crakjie commented Nov 26, 2018

I had updated MR to follow recomendation and rebase with upstream (as of 26/11/2018).

@jvican
Copy link
Contributor

jvican commented Nov 27, 2018

@crakjie Thanks for your work Etienne. I had to change quite a big part of your PR after I had a closer look at the best way of deal with proxy settings. I've added scaladocs and improved the FAQ entry to reflect what the current configuration requirements are.

Please have a look and let me know if this works for you (this approach is more conservative but i think covers most of the use cases reasonably well while reducing the danger that things go wrong in non-reproducible ways).

Etienne Couritas and others added 3 commits November 28, 2018 09:26
And switch to https instead of git for the submodules.

Fixes scalacenter#698.
Make several changes in the code to add a new policy for the way we
handle proxy settings in bloop. This new policy hopes to reduce proxy
configuration problems in different clients that run in different
environments in the same machine.
We seem to get some spurious errors in CI after some changes in latest
master. These errors look related to caching the test resources.

With this commit, the user can add `FORCE_TEST_RESOURCES_GENERATION` to
the commit message and the CI will force the generation of the test
resources.
Copy link
Contributor

@jvican jvican left a comment

Choose a reason for hiding this comment

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

@crakjie I'm merging this so that I can get started with the 1.1.0 release 😄 feel free to have a look offline and still report anything you see in it, in case my changes are not enough for your use case.

@jvican jvican merged commit 3808bf0 into scalacenter:master Nov 28, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A defect or misbehaviour. community Any change or proposal that is contributed by the Open Source Community. docs ergonomics Any change that affects developer ergonomics and the easiness of use of bloop.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants