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

Http proxy for fetch #4676

Merged
merged 2 commits into from
Sep 22, 2022
Merged

Http proxy for fetch #4676

merged 2 commits into from
Sep 22, 2022

Conversation

zicklag
Copy link
Contributor

@zicklag zicklag commented Sep 8, 2022

Changes

  • Adds support for using fetch behind an HTTP proxy.
  • Users can now configure the http proxy through the HTTP_PROXY, HTTPS_PROXY, and NO_PROXY environment variables.

Testing

There are no additional tests. We could potentially add tests, but it would require standing up an HTTP proxy to test through during the test, which might be non-trivial.

It would be up to your discernment whether or not we need to pursue that.

Docs

We should add a section to the docs about use behind HTTP proxies. Especially because most things that support HTTP proxies will read both the uppercase environment variables, and the lowercase environment variables, but the global-agent package I used to support proxies here only supports the uppercase environment variables, so that could be a gotcha, for some people. ( It almost was for me. )

@zicklag
Copy link
Contributor Author

zicklag commented Sep 8, 2022

CI is failing on main for me, so that doesn't seem to be caused by this PR directly. I don't know why CI isn't having an issue on other commits before this PR, though, if I look around at other PRs and the commit on main right now.

@matthewp
Copy link
Contributor

matthewp commented Sep 8, 2022

@zicklag main is passing now, so if you want to rebase your branch it should be good, sorry about that.

@zicklag zicklag closed this Sep 8, 2022
@zicklag zicklag force-pushed the http-proxy-for-fetch branch from 7a4e2a1 to 9290b24 Compare September 8, 2022 16:56
@changeset-bot
Copy link

changeset-bot bot commented Sep 8, 2022

🦋 Changeset detected

Latest commit: baecdfa

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@astrojs/webapi Minor
astro-scripts Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@zicklag zicklag reopened this Sep 8, 2022
@zicklag
Copy link
Contributor Author

zicklag commented Sep 8, 2022

Sorry for the close, I accidentally pushed main to my branch without changes.

Just rebased, and no problem at all about the failures. :D

Use `global-agent` to support HTTP_PROXY, HTTPS_PROXY, and NO_PROXY
environment variables.
@zicklag zicklag force-pushed the http-proxy-for-fetch branch from c76e465 to baecdfa Compare September 12, 2022 19:47
@matthewp matthewp merged commit 5e4c525 into withastro:main Sep 22, 2022
@zicklag zicklag deleted the http-proxy-for-fetch branch September 22, 2022 18:48
@zicklag
Copy link
Contributor Author

zicklag commented Sep 22, 2022

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants