Skip to content

Use faraday #55

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

Merged
merged 10 commits into from
Mar 20, 2019
Merged

Use faraday #55

merged 10 commits into from
Mar 20, 2019

Conversation

prcongithub
Copy link
Contributor

Replaces httparty with Faraday to support multi threaded systems.
HTTParty uses class variables to define host for connections.
These class variables were modified by fcm to make requests to multiple google api end points.
With Faraday we can use connection instance variables to connect to different api end points and not worry about multiple threads making api calls.

@kashif
Copy link
Contributor

kashif commented Feb 24, 2019

🙏thanks @prcongithub will review ASAP

@prcongithub
Copy link
Contributor Author

Any update here?

@kashif kashif merged commit d7ccf74 into decision-labs:master Mar 20, 2019
@kashif
Copy link
Contributor

kashif commented Mar 20, 2019

thanks will push a new version out asap

@@ -21,5 +21,5 @@ Gem::Specification.new do |s|
s.executables = `git ls-files -- bin/*`.split("\n").map{ |f| File.basename(f) }
s.require_paths = ["lib"]

s.add_runtime_dependency('httparty', '~> 0.10', '>= 0.10.0')
s.add_runtime_dependency('faraday','0.15.4')
Copy link

Choose a reason for hiding this comment

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

The dependency can be too strict for the apps that already using faraday in their Gemfile.

ghiculescu added a commit to ghiculescu/fcm that referenced this pull request Dec 23, 2020
Since decision-labs#55 you can't pass client options anymore (well you can but they aren't used for anything).

This results in decision-labs#84 which I'm +1 on, but while it's not resolved, better to not imply anything in the docs.
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.

4 participants