Faraday 2 support for opensearch-transport 2

[BrianHawley]

• Support Faraday 1.x and 2.x in opensearch-transport.
• Made client autodetection not break if the detected HTTP client is
  there but the associated Faraday adapter is not.
• Made the test setup require the referenced adapters.
• Made it more clear in the README that Faraday uses adapters.

[Fixes #59]

Description

This makes it possible to use opensearch-transport with Faraday 1 or 2.

Issues Resolved

Closes [#59] for opensearch-transport 2.x.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[dblock]

See my comments in #75, get this PR in first on main, and then we can backport to 1.x once it's here?

[BrianHawley]

@dblock I forward-ported the fixes from #75 to here. Let me know if they're OK, or need squashing.

[dblock]

This works for me and I'll merge on green, thank you.

If you want to improve upon this, consider changing how this faraday version is set for tests in GHA to avoid a huge complicated matrix of things. I would:

1. Add gem 'faraday', ENV['FARADAY_VERSION'], require: false if ENV.key?('FARADAY_VERSION') as you did, but remove everything else.
2. Create a new GHA workflow that sets FARADAY_VERSION in it, and that runs some minimal integration tests with both 1 and 2.

Alternatively, use https://github.com/thoughtbot/appraisal that makes testing very explicit and generates multiple Gemfile's.

[dblock]

Use an explicit Faraday::VERSION comparison to avoid rescue.

[dblock]

Looks like link checker is also failing, https://github.com/lostisland/faraday-typhoeus is not valid.

[BrianHawley]

@dblock I made the requested changes.

[dblock]

@dblock I made the requested changes.

Looks like some integration tests are failing, check those out? I'll keep clicking "run build" for you as needed.

https://github.com/opensearch-project/opensearch-ruby/runs/7121455260?check_suite_focus=true

Fetching gem metadata from https://rubygems.org/.......
Resolving dependencies...
Bundler could not find compatible versions for gem "faraday":
  In Gemfile:
    faraday (~> 1.0)
    faraday-typhoeus was resolved to 0.2.0, which depends on
      faraday (~> 2.0)
rake aborted!
Command failed with status (6): [cd /home/runner/work/opensearch-ruby/opens...]
/home/runner/work/opensearch-ruby/opensearch-ruby/Rakefile:96:in `block (3 levels) in <top (required)>'
/home/runner/work/opensearch-ruby/opensearch-ruby/Rakefile:94:in `each'
/home/runner/work/opensearch-ruby/opensearch-ruby/Rakefile:94:in `block (2 levels) in <top (required)>'
Tasks: TOP => bundle:install

[BrianHawley]

@dblock I moved the Faraday 1 tests to their own job, as requested.

I think the integration test problem was from faraday-typhoeus getting included during the Faraday 1 tests of Ruby 2.7.0 or higher. I put a constraint on that. Next test run will show if it worked.

I agree that something like appraisal would be best to use in the long run, and would be up for doing that conversion later. Not a fan of gemspecs with if or unless in them.

As for the faraday-typhoeus problem with Ruby 2.6 and JRuby (which is 2.6 compatible), I'm going to make a PR to that repo to fix it. Later on, if that PR is merged, I can clean things up here. It's still not going to support Faraday 1, but Faraday has built-in support for Typhoeus in that version.

[dblock]

Looks like you may need to wrap RUBY_VERSION into Gem::Version.new(). Btw, you can enable GHA on your fork and see all these failures.

[BrianHawley]

@dblock weird, I didn't need to wrap RUBY_VERSION in Gem::Version.new() locally. Must be an older RubyGems thing.

In any case, doing so made it possible to make the conditions read better. Updated.

[BrianHawley]

Btw, you can enable GHA on your fork and see all these failures.

I haven't audited the actions to make sure that they all apply to my case, of not being someone who releases these gems or sets tags and such.

I tried to run the suite locally with https://github.com/nektos/act but several of the server setup steps don't really work in containers, even privileged ones, such as the "Increase system limits" steps or something in the Opendistro setup action. I was going to try to diagnose this later and get the suite running locally with act.

[dblock]

I tried to run the suite locally with https://github.com/nektos/act but several of the server setup steps don't really work in containers, even privileged ones, such as the "Increase system limits" steps or something in the Opendistro setup action. I was going to try to diagnose this later and get the suite running locally with act.

Yeah act only gets you that far. But you can also just enable actions on your own fork. I think you go to https://github.com/BrianHawley/opensearch-ruby settings and it's a checkbox somewhere under Actions. That said, once this is merged GH won't think you're a noob anymore and will run your jobs on PRs. It's just being extra paranoid :)

[dblock]

This LGTM, @VachaShah care to be the second reviewer?

[VachaShah]

LGTM! Thank you very much @BrianHawley for adding this support :)

[VachaShah]

@dblock @BrianHawley Does this need to backported to both 2.0 and 1.x?

[BrianHawley]

@VachaShah yes, both 2.0 and 1.x.

It should be a simple cherry-pick to do so with no conflicts. I actually had another PR targeting 1.x (#75) but this is the one I got the testing done on. I gather from a comment by @dblock in that one that backport PRs are done automatically, so that process should be fine.

[BrianHawley]

@VachaShah any chance of new opensearch-transport releases with this change soon? Maybe after #89 and #90 are merged?

[dblock]

@BrianHawley just open an issue when you want a release so we can make these visible to a larger group

I see #48 that should enable this project to release very often, too.

[BrianHawley]

@dblock OK, I created #91.