Skip to content

Commit

Permalink
Add default open_timeout
Browse files Browse the repository at this point in the history
Faraday supports this and passes it through to the underlying Net::HTTP
interface. There's little to no reason not to specify something
reasonable here. If clients can't connect in 1000ms, there are likely
some problems with the network in one fashion or another.
  • Loading branch information
bhornseth committed Feb 27, 2019
1 parent 13b596b commit 073bccc
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 2 deletions.
1 change: 1 addition & 0 deletions CHANGELOG
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ v2.0.0
- Upgrade to Rspec 3
- Upgrade all dependencies to current stable versions
- Catch MultiJson::ParseErrors and return SparkApi::InvalidJSON error instead
- Add default 1 second open_timeout to HTTP requests

v1.4.28
- move public_suffix to development dependency where it belongs
Expand Down
4 changes: 3 additions & 1 deletion lib/spark_api/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ module Configuration
# valid configuration options
VALID_OPTION_KEYS = [:api_key, :api_secret, :api_user, :endpoint,
:user_agent, :version, :ssl, :ssl_verify, :oauth2_provider, :authentication_mode,
:auth_endpoint, :callback, :compress, :timeout, :middleware, :dictionary_version, :request_id_chain].freeze
:auth_endpoint, :callback, :compress, :open_timeout, :timeout, :middleware, :dictionary_version, :request_id_chain].freeze
OAUTH2_KEYS = [:authorization_uri, :access_uri, :client_id, :client_secret,
# Requirements for authorization_code grant type
:redirect_uri,
Expand Down Expand Up @@ -42,6 +42,7 @@ module Configuration
DEFAULT_SSL_VERIFY = true
DEFAULT_OAUTH2 = nil
DEFAULT_COMPRESS = false
DEFAULT_OPEN_TIMEOUT = 1 # seconds
DEFAULT_TIMEOUT = 5 # seconds
DEFAULT_MIDDLEWARE = 'spark_api'
DEFAULT_DICTIONARY_VERSION = nil
Expand Down Expand Up @@ -78,6 +79,7 @@ def reset_configuration
self.ssl_verify = DEFAULT_SSL_VERIFY
self.version = DEFAULT_VERSION
self.compress = DEFAULT_COMPRESS
self.open_timeout = DEFAULT_OPEN_TIMEOUT
self.timeout = DEFAULT_TIMEOUT
self.middleware = DEFAULT_MIDDLEWARE
self.dictionary_version = DEFAULT_DICTIONARY_VERSION
Expand Down
1 change: 1 addition & 0 deletions lib/spark_api/connection.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ def connection(force_ssl = false)

conn = Faraday.new(opts) do |conn|
conn.response self.middleware.to_sym
conn.options[:open_timeout] = self.open_timeout
conn.options[:timeout] = self.timeout
conn.adapter Faraday.default_adapter
end
Expand Down
14 changes: 13 additions & 1 deletion spec/unit/spark_api/configuration_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
expect(SparkApi.timeout).to eq(5)
expect(SparkApi.request_id_chain).to be_nil
expect(SparkApi.middleware).to eq('spark_api')
expect(SparkApi.open_timeout).to eq(1)
end
end

Expand All @@ -25,6 +26,7 @@
:api_user => "1234",
:auth_endpoint => "https://login.wade.dev.fbsdata.com",
:endpoint => "http://api.wade.dev.fbsdata.com",
:open_timeout => 2,
:timeout => 15,
:request_id_chain => 'foobar')

Expand All @@ -34,6 +36,7 @@
expect(client.auth_endpoint).to match("https://login.wade.dev.fbsdata.com")
expect(client.endpoint).to match("http://api.wade.dev.fbsdata.com")
expect(client.version).to match("v1")
expect(client.open_timeout).to eq(2)
expect(client.timeout).to eq(15)
expect(client.request_id_chain).to eq('foobar')
end
Expand Down Expand Up @@ -97,6 +100,7 @@
config.version = "veleventy"
config.endpoint = "test.api.sparkapi.com"
config.user_agent = "my useragent"
config.open_timeout = 2
config.timeout = 15
end

Expand All @@ -107,6 +111,7 @@
expect(SparkApi.endpoint).to match("test.api.sparkapi.com")
expect(SparkApi.user_agent).to match("my useragent")
expect(SparkApi.oauth2_enabled?()).to be false
expect(SparkApi.open_timeout).to eq(2)
expect(SparkApi.timeout).to eq(15)
end

Expand Down Expand Up @@ -226,14 +231,21 @@
expect(c.connection.headers["Accept-Encoding"]).to eq("gzip, deflate")
end

it "should set default timeout of 5 seconds" do
it "should set default read timeout of 5 seconds" do
c = SparkApi::Client.new(:endpoint => "https://sparkapi.com")
expect(c.connection.options[:timeout]).to eq(5)
end

it "should set default open timeout of 1 second" do
c = SparkApi::Client.new(:endpoint => "https://sparkapi.com")
expect(c.connection.options[:open_timeout]).to eq(1)
end

it "should set alternate timeout if specified" do
c = SparkApi::Client.new(:endpoint => "https://sparkapi.com",
:open_timeout => 5,
:timeout => 15)
expect(c.connection.options[:open_timeout]).to eq(5)
expect(c.connection.options[:timeout]).to eq(15)
end
end
Expand Down

0 comments on commit 073bccc

Please sign in to comment.