support proxies through environment variables #235
Conversation
IMO, this isn't breaking. It is a bug fix... |
|
|
||
| ### Proxy | ||
|
|
||
| Proxies are supported through `https_proxy`, `http_proxy`, and `no_proxy` environment variables: |
There was a problem hiding this comment.
Let's have these in the README as both SCREAMING_SNAKE_CASE and lower case, or otherwise make it clearer that we are operating insensitively.
I tried to add tests. But struggled with creating a proxy to use in the tests. I tried to write a simple proxy myself using I don't have much experience working with proxies to be honest. Maybe there is an architectural issue with what I'm trying? Here is my latest attempt using QUnit.module('proxy', async function (hooks) {
hooks.beforeEach(async function () {
const port = await getPort();
httpProxy
.createProxyServer({
target: process.env.EMBER_SOURCE_CHANNEL_URL_HOST,
})
.listen(port);
this.proxyPort = port;
});
hooks.afterEach(async function () {
// clean-up proxy environment variables
Object.keys(process.env).forEach((envVariable) => {
const proxyEnvVariables = ['http_proxy', 'https_proxy', 'no_proxy'];
const isProxyEnvVariable = proxyEnvVariables.includes(envVariable.toLocaleLowerCase());
if (isProxyEnvVariable) {
delete process.env[envVariable];
}
});
// shutdown proxy
await this.proxy.close();
});
['http_proxy', 'https_proxy'].forEach((envVariable) => {
QUnit.only(`supports ${envVariable} environment variable`, async function (assert) {
const { assetPath, proxyPort, server } = this;
// arrange: set proxy environment variable
process.env[envVariable] = `http://localhost:${proxyPort}`;
// act
const actual = await getChannelURL('canary');
// assert
const expected = `http://${server.host}:${server.port}/builds.emberjs.com${assetPath}`;
assert.equal(actual, expected);
});
});
});Any idea? |
This adds support for
https_proxy,http_proxy, andno_proxyenvironment variables. If these are set, a proxy will be used for network requests. This is important for some corporate networks, which require proxy usage for network requests to public internet.Most environment variables are uppercase. But for
https_proxy,http_proxy, andno_proxylowercase is more commonly used. There are some programs (e.g.wget), which support lowercase but not uppercase. The implementation proposed here is case-insensitive.This change enables running ember-try scenarios using a proxy. The other tools used by ember-try supports these environment variables already. Only ember-source-channel-url does not support proxy usage yet at all.
Not sure if it should be considered a breaking changes. It changes behavior if proxy environment variables are set. But you could argue that a consumer, which sets the environment variables expects a program to use a proxy.
Closes #4