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

Disregard proxy environment variables in yesod devel #945

Merged
merged 3 commits into from Mar 1, 2015

Conversation

Projects
None yet
2 participants
@paul-rouse
Contributor

paul-rouse commented Feb 28, 2015

After the change to proxy settings in http-client (snoyberg/http-client#94) the yesod devel reverse proxy now attempts to connect to the application via any further proxy specified by the environment variables. I can't see any reason why the new behaviour might be useful in this specific case, so it seems better to me to force no proxy - am I missing something?

@snoyberg

This comment has been minimized.

Show comment
Hide comment
@snoyberg

snoyberg Mar 1, 2015

Member

Looks good, but we need a lower bound on http-client for that new function.

Member

snoyberg commented Mar 1, 2015

Looks good, but we need a lower bound on http-client for that new function.

@paul-rouse

This comment has been minimized.

Show comment
Hide comment
@paul-rouse

paul-rouse Mar 1, 2015

Contributor

Good point (blush) - lower bound now added. Following on, though, do you think it is OK to break compatibility with earlier http-client versions?

Contributor

paul-rouse commented Mar 1, 2015

Good point (blush) - lower bound now added. Following on, though, do you think it is OK to break compatibility with earlier http-client versions?

@snoyberg

This comment has been minimized.

Show comment
Hide comment
@snoyberg

snoyberg Mar 1, 2015

Member

It's a "nice to have." Probably all that would be necessary is a quick MIN_VERSION_http_client(0,4,7) that simply doesn't alter this setting for older versions. I'm fine with or without that compatibility. I'll hold off on merging till I hear whether you want to add the compatibility back or not.

Member

snoyberg commented Mar 1, 2015

It's a "nice to have." Probably all that would be necessary is a quick MIN_VERSION_http_client(0,4,7) that simply doesn't alter this setting for older versions. I'm fine with or without that compatibility. I'll hold off on merging till I hear whether you want to add the compatibility back or not.

@paul-rouse

This comment has been minimized.

Show comment
Hide comment
@paul-rouse

paul-rouse Mar 1, 2015

Contributor

Compatibility done with CPP conditionals. I'll try to remember this needs tidying up after a reasonable period...

Contributor

paul-rouse commented Mar 1, 2015

Compatibility done with CPP conditionals. I'll try to remember this needs tidying up after a reasonable period...

snoyberg added a commit that referenced this pull request Mar 1, 2015

Merge pull request #945 from paul-rouse/master
Disregard proxy environment variables in yesod devel

@snoyberg snoyberg merged commit a2da368 into yesodweb:master Mar 1, 2015

1 check was pending

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
@snoyberg

This comment has been minimized.

Show comment
Hide comment
@snoyberg

snoyberg Mar 1, 2015

Member

Perfect!

Member

snoyberg commented Mar 1, 2015

Perfect!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment