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

Add proxy setting when url_prefix is changed #1276

Merged
merged 2 commits into from Apr 28, 2021

Conversation

ci
Copy link
Contributor

@ci ci commented Apr 27, 2021

Description

Without this change, setting the url_prefix after a Faraday::Connection is initialized results in no_proxy settings ignored when using relative paths. This would be one of the "proposed" fixes in #1275.

Fixes #1275

Todos

Without this setting, setting the url_prefix after
a Faraday::Connection is initialized results in no_proxy
settings ignored when using relative paths.
Copy link
Member

@olleolleolle olleolleolle left a comment

Choose a reason for hiding this comment

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

I think this fix and test look neat! Thank you!

Copy link
Member

@iMacTia iMacTia left a comment

Choose a reason for hiding this comment

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

Thanks for raising and fixing this @ci.
This issue also highlights why it's not a good idea to change the url_prefix on an established connection, I'd be curious to know why that's done 🤔

Anyway, this looks good and adds a new test. LGTM 🎉 !

@iMacTia
Copy link
Member

iMacTia commented Apr 28, 2021

@ci feel free to simply increase the limit for Metrics/ClassLength in the .rubocop_todo.yml

@ci
Copy link
Contributor Author

ci commented Apr 28, 2021

@ci feel free to simply increase the limit for Metrics/ClassLength in the .rubocop_todo.yml

Woops, missed that, thanks @iMacTia! Any preference for the ClassLength? I've set it to 250 (which is still +10 than the current size)

@olleolleolle olleolleolle merged commit 53e4bae into lostisland:main Apr 28, 2021
@olleolleolle
Copy link
Member

@ci Thank you for taking the time to describe and fix this issue!

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.

no_proxy is ignored for relative paths when changing url_prefix
3 participants