Skip to content

Conversation

@janko
Copy link

@janko janko commented Feb 10, 2015

I'm wanted to use Typhoeus for Elasticsearch, but I got The option: disable_ssl_peer_verification is invalid. (Ethon::Errors::InvalidOption). This is because I forgot to require the official adapter (typhoeus/adapters/faraday). I was thinking that it would be make sense if it was automatically required, so that it works out-of-the-box.

By the way, I started using Elasticsearch only a week ago, and I'm really loving the separation of gems you guys made for Elasticsearch, it's just so easy to find everything.

@janko janko force-pushed the typhoeus-out-of-the-box branch from d6a9899 to 71d4245 Compare February 10, 2015 13:44
@karmi
Copy link
Contributor

karmi commented Feb 10, 2015

@janko-m I'm not a huge fan of require-ing third-party stuff from within methods and conditionals, but I see your point... Let me chew on it for a while, and please ping me if I let it slip!

@karmi karmi added the feature label Feb 10, 2015
@janko janko force-pushed the typhoeus-out-of-the-box branch from 71d4245 to 34fb0b7 Compare February 10, 2015 15:36
@karmi
Copy link
Contributor

karmi commented Apr 24, 2015

@janko-m Hi, sorry for "chewing" on it for so long :) Even after some time, I'd rather risk somebody makes the mistake you did, than requiring files from within conditionals within methods... I had some really bad experiences with that when some libraries did that...

@karmi karmi closed this Apr 24, 2015
@janko janko deleted the typhoeus-out-of-the-box branch April 24, 2015 12:35
@janko
Copy link
Author

janko commented Apr 24, 2015

Sure, I understand, you already mention require "typheous/adapters/faraday" in the readme anyway.

@karmi
Copy link
Contributor

karmi commented Apr 24, 2015

Great!, thanks for understanding!

rafayqayyum pushed a commit to rafayqayyum/elasticsearch-ruby that referenced this pull request Mar 4, 2025
rafayqayyum pushed a commit to rafayqayyum/elasticsearch-ruby that referenced this pull request Mar 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants