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

tests: bypass the proxy if testing DNS override #2341

Merged
merged 1 commit into from
Jul 3, 2024

Conversation

schopin-pro
Copy link
Contributor

If an explicit proxy is configured in the environment, then the request will go through it rather than actually resolving the domain. Either we're hitting the target domain on a weird port which will likely fail, or the proxy straight up denies that weird request.

To work around this, we actually modify the environment variables to make sure that our target domain is in the exception list for the proxy.

We're hitting this issue in the Ubuntu CI. Amazingly enough, the tests actually passed once there, although the exact circumstances that allowed this are still a bit of a mystery.

@seanmonstar
Copy link
Owner

Could those tests just set ClientBuilder::no_proxy(), instead of modifying the environment?

@schopin-pro
Copy link
Contributor Author

Sure, that sounds a lot smarter 😁

If an explicit proxy is configured in the environment, then the request
will go through it rather than actually resolving the domain. Either
we're hitting the target domain on a weird port which will likely fail,
or the proxy straight up denies that weird request.

We're hitting this issue in the Ubuntu CI. Amazingly enough, the tests
actually passed *once* there, although the exact circumstances that
allowed this are still a bit of a mystery.
@seanmonstar seanmonstar merged commit 892569e into seanmonstar:master Jul 3, 2024
36 checks passed
@schopin-pro schopin-pro deleted the no-proxy branch July 3, 2024 15:40
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.

None yet

2 participants