feat: Allow lower Faraday versions in gemspec #16
Conversation
groq.gemspec
Outdated
| @@ -33,7 +33,7 @@ Gem::Specification.new do |spec| | |||
| spec.executables = spec.files.grep(%r{\Aexe/}) { |f| File.basename(f) } | |||
| spec.require_paths = ["lib"] | |||
|
|
|||
| spec.add_dependency "faraday", "~> 2.0" | |||
| spec.add_dependency "faraday", "~> 1.0" | |||
There was a problem hiding this comment.
Can we make this ">= 1.0" to allow 2.X as well?
There was a problem hiding this comment.
Any ideas how we might test both faraday 1.X and faraday 2.X in CI?
There was a problem hiding this comment.
Hi thank you! Yes I have updated this CR to allow for 2.x as well ✅
As for the latter I will push a new commit shortly editing main.yml workflow in a minute. The approach would be, IMO, to have the faraday version pulled from env var. In your workflow if the "FARADAY_VERSION" var is not set, it will default to the latest version as per Gemfile; if it is set it will pull the custom version.
- name: Run the test suite with lower Faraday versions
run: FARADAY_VERSION=1.0.0 bin/setup && bundle exec rake test
There was a problem hiding this comment.
Let me know if this works / makes sense - I cannot run CI without workflow approval so I hope this helps :)
|
@drnic nice callout - FWIW all sum I went ahead and edited the CI file to have the possibility of specifying the version with an Env var I tested the command below - tests fail below v 1.10.0, pass above that for V1. On V2 tests pass for all V2 versions >= V 2.0.1 - 2.0.0 does not specify a default adapter. So it looks like if the gemspec says "all versions >1.10.0" work", 2.0.0 itself does not, but everything above does. I am not sure how to exclude that one version from the spec - may be that allowing lower versions has that issue, though I'd note now that the gemspec currently allows 2.0.0 maybe and it wouldn't work :)
|
|
Good work! |
In the issue #15 I had reported that, due to Faraday 2.0's back compatibility conflicts with Faraday 1.0, many projects that do have hard requirements on the latter will have some dependency issues trying to install groq-ruby when they cannot utilize Faraday 2.0.
A simple proposed fix is to allow the gemspec to pessimistic lock to version 1.0, so if a given consumer's bundle depends on Faraday < 2, it will still allow install of Groq-Ruby without dependency clash. This assumes the library will function as intended on either HTTP client version.
To validate that, I read the test suite - key features like error handling and response streaming should be supported in either version of Faraday's HTTP Client offerings. Since the test suite covers the behaviors being abstracted from the underlying HTTP client, I went ahead and tested the library here with
rake testas prescribed, in a fresh rbenv with ruby 3.1.4 both when forcing Faraday v1 and v2:It seems like this would solve my issue and not impact the library here - if you have any questions for follow up let me know, I am happy to contribute :)