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.

Sign up for GitHub

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

Faraday retry middleware #490

Merged
merged 9 commits into from
Mar 30, 2021
Merged

Faraday retry middleware #490

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
e506adc
faraday retry middleware
timothysmith0609 Feb 25, 2021
79c6702
terser test
timothysmith0609 Feb 25, 2021
3d9934b
cop
timothysmith0609 Feb 25, 2021
02e6443
more general purpose approach
timothysmith0609 Mar 2, 2021
2e93c98
remove bad file
timothysmith0609 Mar 2, 2021
c8deae8
always use the default faraday config, even if more is added
timothysmith0609 Mar 3, 2021
1a75767
syntax/test fixes
timothysmith0609 Mar 3, 2021
980dcd0
clearer name
timothysmith0609 Mar 3, 2021
eb673a1
README
timothysmith0609 Mar 26, 2021
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
6 changes: 6 additions & 0 deletions lib/kubeclient/common.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,10 @@ def initialize_client(
end
end

def with_faraday_config(&block)
Copy link
Contributor

Choose a reason for hiding this comment

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

it looks like this could be called multiple times, but it can't ... so maybe call it replace_faraday_config
... and would seem more intuitive to specify this during initalization anyway like configure_faraday: -> {|c| stuff }

@faraday_client = create_faraday_client(&block)
end

Copy link
Contributor Author

@timothysmith0609 timothysmith0609 Mar 2, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

I've changed my approach following your suggestion to expose this method which takes a block that yields the Faraday::Connection object, which is really the only place we can specify all the middlewares and other config. Let me know if you think this is on the right track. (Once we settle on an approach I'll do the relevant README updates as well)

Copy link
Collaborator

Choose a reason for hiding this comment

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

nice 👍

def method_missing(method_sym, *args, &block)
if discovery_needed?(method_sym)
discover
Expand Down Expand Up @@ -313,6 +317,8 @@ def create_faraday_client(url = nil)
if @auth_options[:username] && @auth_options[:password]
connection.basic_auth(@auth_options[:username], @auth_options[:password])
end
# hook for adding custom faraday configuration
yield(connection) if block_given?
connection.use(FaradayMiddleware::FollowRedirects, limit: @http_max_redirects)
connection.response(:raise_error)
end
Expand Down
13 changes: 13 additions & 0 deletions test/test_kubeclient.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,19 @@ def test_entity_list_parsed_symbolized
assert_equal(%i[metadata spec status], response[:items].first.keys)
end

def test_custom_faraday_config_options
client = Kubeclient::Client.new('http://localhost:8080/api/', 'v1')
expected_middlewares = [FaradayMiddleware::FollowRedirects, Faraday::Response::RaiseError]
expected_middlewares.each do |klass|
assert(client.faraday_client.builder.handlers.include?(klass))
end
client.with_faraday_config { |connection| connection.use(Faraday::Request::Retry) }
expected_middlewares << Faraday::Request::Retry
expected_middlewares.each do |klass|
assert(client.faraday_client.builder.handlers.include?(klass))
end
end

class ServiceList
attr_reader :names

Expand Down