Be able to set an SSL verify mode on faraday #982
Conversation
taquitos
commented
Jan 24, 2018
taquitos
commented
- addresses issue Can't set SSL verify mode in faraday options #981
- addresses issue #981
lib/octokit/default.rb
Outdated
| # Default SSL verify mode from ENV | ||
| # @return [String] | ||
| def ssl_verify_mode | ||
| ENV['OCTOKIT_SSL_VERIFY_MODE'] |
There was a problem hiding this comment.
So because most folks will not have this set, it will default to false which is a breaking change.
See https://github.com/lostisland/faraday/blob/923f90558c22326c9a757a7ff9dfb3a006d0abc8/lib/faraday/adapter/net_http.rb#L124-L130 for the implementation details
What do you think about doing something like this instead?
ENV.fetch("OCTOKIT_SSL_VERIFY_MODE") { true }There was a problem hiding this comment.
That makes sense, totally forgot about defaults.
| end | ||
| conn = Octokit.client.send(:agent).instance_variable_get(:"@conn") | ||
| expect(conn.ssl[:verify_mode]).to eq(OpenSSL::SSL::VERIFY_NONE) | ||
| end |
There was a problem hiding this comment.
Can we add a test here to ensure the expected default remains unchanged?
There was a problem hiding this comment.
Of course, good call
update spec to add default ssl value checking
|
Alright, all up-to-date 🎉 |
|
So great to see this merged 🎉 Thanks! |
|
This has been released as part of https://github.com/octokit/octokit.rb/releases/tag/v4.9.0 |
|
Awesome, thanks for keeping us posted 👍 |
