Skip to content
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

Allow Faraday v2 usage #1411

Merged
merged 3 commits into from
May 26, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions .github/workflows/octokit.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,17 +20,23 @@ jobs:
fail-fast: false
matrix:
os: [ ubuntu ]
ruby: [ 2.5, 2.6, 2.7, '3.0', 3.1, head ]
ruby: [ 2.5, 2.6, 2.7, '3.0', '3.1', head ]
faraday: [ '~> 1.0', '~> 2.0' ]
exclude:
- ruby: 2.5
faraday: '~> 2.0'
env:
FARADAY_VERSION: ${{ matrix.faraday }}

steps:
- uses: actions/checkout@v2
- name: Cache Ruby dependencies
uses: actions/cache@v1
with:
path: ./.bundle/gems
key: cache-${{ runner.os }}-${{ matrix.ruby }}-${{ github.sha }}
key: cache-${{ runner.os }}-${{ matrix.ruby }}-${{ matrix.faraday }}-${{ github.sha }}
restore-keys: |
cache-${{ runner.os }}-${{ matrix.ruby }}-
cache-${{ runner.os }}-${{ matrix.ruby }}-${{ matrix.faraday }}-
- name: Set up Ruby
uses: ruby/setup-ruby@v1
with:
Expand Down
4 changes: 4 additions & 0 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ group :test do
gem 'webmock', '~> 3.8', '>= 3.8.2'
end

gem 'faraday', ENV.fetch('FARADAY_VERSION', '~> 2.0')
gem 'faraday-retry'
gem 'faraday-multipart'

group :test, :development do
gem 'pry-byebug'
gem 'redcarpet'
Expand Down
10 changes: 6 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -603,7 +603,7 @@ traffic:

```ruby
stack = Faraday::RackBuilder.new do |builder|
builder.use Faraday::Request::Retry, exceptions: [Octokit::ServerError]
builder.use Faraday::Retry::Middleware, exceptions: [Octokit::ServerError] # or Faraday::Request::Retry for Faraday < 2.0
builder.use Octokit::Middleware::FollowRedirects
builder.use Octokit::Response::RaiseError
builder.use Octokit::Response::FeedParser
Expand Down Expand Up @@ -740,9 +740,11 @@ when writing new specs.
This library aims to support and is [tested against][actions] the following Ruby
implementations:

- Ruby 2.5
- Ruby 2.6
- Ruby 2.7
* Ruby 2.5
* Ruby 2.6
* Ruby 2.7
* Ruby 3.0
* Ruby 3.1

If something doesn't work on one of these Ruby versions, it's a bug.

Expand Down
8 changes: 8 additions & 0 deletions lib/octokit/authentication.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,14 @@ module Octokit
# Authentication methods for {Octokit::Client}
module Authentication

# In Faraday 2.x, the authorization middleware uses new interface
FARADAY_BASIC_AUTH_KEYS =
if Gem::Version.new(Faraday::VERSION) >= Gem::Version.new('2.0')
[:authorization, :basic]
else
[:basic_auth]
end

# Indicates if the client was supplied Basic Auth
# username and password
#
Expand Down
2 changes: 1 addition & 1 deletion lib/octokit/client.rb
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ def client_without_redirects(options = {})
conn_opts[:ssl] = { :verify_mode => @ssl_verify_mode } if @ssl_verify_mode
conn = Faraday.new(conn_opts) do |http|
if basic_authenticated?
http.request :basic_auth, @login, @password
http.request *FARADAY_BASIC_AUTH_KEYS, @login, @password
elsif token_authenticated?
http.request :authorization, 'token', @access_token
elsif bearer_authenticated?
Expand Down
2 changes: 1 addition & 1 deletion lib/octokit/client/pub_sub_hubbub.rb
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ def pub_sub_hubbub_request(options = {})
conn = Faraday.new(:url => @api_endpoint) do |http|
http.headers[:user_agent] = user_agent
if basic_authenticated?
http.request :basic_auth, @login, @password
http.request *FARADAY_BASIC_AUTH_KEYS, @login, @password
elsif token_authenticated?
http.request :authorization, 'token', @access_token
end
Expand Down
4 changes: 2 additions & 2 deletions lib/octokit/connection.rb
Original file line number Diff line number Diff line change
Expand Up @@ -107,13 +107,13 @@ def agent
http.headers[:content_type] = "application/json"
http.headers[:user_agent] = user_agent
if basic_authenticated?
http.request :basic_auth, @login, @password
http.request *FARADAY_BASIC_AUTH_KEYS, @login, @password
elsif token_authenticated?
http.request :authorization, 'token', @access_token
elsif bearer_authenticated?
http.request :authorization, 'Bearer', @bearer_token
elsif application_authenticated?
http.request :basic_auth, @client_id, @client_secret
http.request *FARADAY_BASIC_AUTH_KEYS, @client_id, @client_secret
end
end
end
Expand Down
20 changes: 15 additions & 5 deletions lib/octokit/default.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,15 @@
require 'octokit/response/raise_error'
require 'octokit/response/feed_parser'
require 'octokit/version'
require 'octokit/warnable'

if Gem::Version.new(Faraday::VERSION) >= Gem::Version.new('2.0')
begin
require 'faraday/retry'
rescue LoadError
Octokit::Warnable.octokit_warn 'To use retry middleware with Faraday v2.0+, install `faraday-retry` gem'
end
end
Comment on lines +7 to +13
Copy link

Choose a reason for hiding this comment

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

Is faraday-retry usable on Faraday 1.x? I wonder if it makes sense to just add it as a dependency if so.


module Octokit

Expand All @@ -20,12 +29,13 @@ module Default
# Default WEB endpoint
WEB_ENDPOINT = "https://github.com".freeze

# In Faraday 0.9, Faraday::Builder was renamed to Faraday::RackBuilder
RACK_BUILDER_CLASS = defined?(Faraday::RackBuilder) ? Faraday::RackBuilder : Faraday::Builder

# Default Faraday middleware stack
MIDDLEWARE = RACK_BUILDER_CLASS.new do |builder|
builder.use Faraday::Request::Retry, exceptions: [Octokit::ServerError]
MIDDLEWARE = Faraday::RackBuilder.new do |builder|
# In Faraday 2.x, Faraday::Request::Retry was moved to a separate gem
# so we use it only when it's available.
builder.use Faraday::Request::Retry, exceptions: [Octokit::ServerError] if defined?(Faraday::Request::Retry)
builder.use Faraday::Retry::Middleware, exceptions: [Octokit::ServerError] if defined?(Faraday::Retry::Middleware)

builder.use Octokit::Middleware::FollowRedirects
builder.use Octokit::Response::RaiseError
builder.use Octokit::Response::FeedParser
Expand Down
8 changes: 8 additions & 0 deletions lib/octokit/response/base_middleware.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
require 'faraday'

module Octokit
module Response
# In Faraday 2.x, Faraday::Response::Middleware was removed
BaseMiddleware = defined?(Faraday::Response::Middleware) ? Faraday::Response::Middleware : Faraday::Middleware
Copy link

Choose a reason for hiding this comment

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

Worth noting that Faraday::Middleware can be used as far back as v1.2.0: lostisland/faraday@d56742a

end
end
4 changes: 2 additions & 2 deletions lib/octokit/response/feed_parser.rb
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
require 'faraday'
require 'octokit/response/base_middleware'

module Octokit

module Response

# Parses RSS and Atom feed responses.
class FeedParser < Faraday::Response::Middleware
class FeedParser < BaseMiddleware

def on_complete(env)
if env[:response_headers]["content-type"] =~ /(\batom|\brss)/
Expand Down
4 changes: 2 additions & 2 deletions lib/octokit/response/raise_error.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
require 'faraday'
require 'octokit/response/base_middleware'
require 'octokit/error'

module Octokit
Expand All @@ -7,7 +7,7 @@ module Response

# This class raises an Octokit-flavored exception based
# HTTP status codes returned by the API
class RaiseError < Faraday::Response::Middleware
class RaiseError < BaseMiddleware

def on_complete(response)
if error = Octokit::Error.from_response(response)
Expand Down
2 changes: 2 additions & 0 deletions lib/octokit/warnable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ module Octokit
# Allows warnings to be suppressed via environment variable.
module Warnable

module_function

# Wrapper around Kernel#warn to print warnings unless
# OCTOKIT_SILENT is set to true.
#
Expand Down
4 changes: 2 additions & 2 deletions octokit.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ require 'octokit/version'

Gem::Specification.new do |spec|
spec.add_development_dependency 'bundler', '>= 1', '< 3'
spec.add_dependency 'sawyer', '>= 0.5.3', '~> 0.8.0'
spec.add_dependency 'faraday', '>= 0.9'
spec.add_dependency 'sawyer', '~> 0.9'
spec.add_dependency 'faraday', '>= 1', '< 3'
spec.authors = ["Wynn Netherland", "Erik Michaels-Ober", "Clint Shryock"]
spec.description = %q{Simple wrapper for the GitHub API}
spec.email = ['wynn.netherland@gmail.com', 'sferik@gmail.com', 'clint@ctshryock.com']
Expand Down
1 change: 1 addition & 0 deletions spec/helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
if Gem::Version.new(RUBY_VERSION) < Gem::Version.new('3.2.0')
require 'pry-byebug'
end
require 'faraday/multipart' if Gem::Version.new(Faraday::VERSION) >= Gem::Version.new('2.0')

WebMock.disable_net_connect!()

Expand Down