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

Add Typheous as our default networking gem #87

Merged
merged 9 commits into from Jul 9, 2018

Conversation

mattrayner
Copy link
Contributor

No description provided.

@coveralls
Copy link

coveralls commented Jul 9, 2018

Coverage Status

Coverage remained the same at 100.0% when pulling 9a87664 on typheous-investigation into 12e42fd on master.

end
end
end
end

Choose a reason for hiding this comment

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

Layout/TrailingBlankLines: 1 trailing blank lines detected.

if endpoint.query
# Returns [ ["key", "value"], ["key", "value"] ]
key_value_array = URI.decode_www_form(endpoint.query)
key_value_array.map! { |key_value_pair| [ key_value_pair[0].to_sym, key_value_pair[1] ] }

Choose a reason for hiding this comment

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

Layout/SpaceInsideArrayLiteralBrackets: Do not use space inside array brackets.

exception_class = Parliament::ClientError
when Net::HTTPServerError # 5xx Status
exception_class = Parliament::ServerError
def separate_uri(query_url, query_params, additional_params)

Choose a reason for hiding this comment

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

Metrics/MethodLength: Method has too many lines. [11/10]

(response.headers&.[]('Content-Length').nil? && response.body.empty?)
elsif /\A4\w{2}/.match(response.code.to_s) # 4xx Status
Parliament::ClientError
elsif /\A5\w{2}/.match(response.code.to_s) # 5xx Status

Choose a reason for hiding this comment

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

Performance/RedundantMatch: Use =~ in places where the MatchData returned by #match will not be used.

exception_class = if response.success? # 2xx Status
Parliament::NoContentResponseError if response.headers&.[]('Content-Length') == '0' ||
(response.headers&.[]('Content-Length').nil? && response.body.empty?)
elsif /\A4\w{2}/.match(response.code.to_s) # 4xx Status

Choose a reason for hiding this comment

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

Performance/RedundantMatch: Use =~ in places where the MatchData returned by #match will not be used.

headers.each do |key, value|
request.add_field(key, value)
end
def headers

Choose a reason for hiding this comment

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

Lint/DuplicateMethods: Method Parliament::Request::BaseRequest#headers is defined at both lib/parliament/request/base_request.rb:15 and lib/parliament/request/base_request.rb:183.

request = Net::HTTP::Post.new(
endpoint_uri.request_uri,
'Content-Type' => 'application/json'
def post(params: nil, body: nil, timeout: TIMEOUT, connecttimeout: CONNECTTIMEOUT)

Choose a reason for hiding this comment

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

Metrics/MethodLength: Method has too many lines. [15/10]

net_response = http.start do |h|
api_request = Net::HTTP::Get.new(endpoint_uri.request_uri)
add_headers(api_request)
def get(params: nil, timeout: TIMEOUT, connecttimeout: CONNECTTIMEOUT)

Choose a reason for hiding this comment

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

Metrics/MethodLength: Method has too many lines. [14/10]

@@ -7,6 +9,9 @@ module Request
# @attr_reader [String] base_url the base url of our api. (expected: http://example.com - without the trailing slash).
# @attr_reader [Hash] headers the headers being sent in the request.
class BaseRequest
TIMEOUT = 40.freeze
CONNECTTIMEOUT = 5.freeze

Choose a reason for hiding this comment

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

Style/RedundantFreeze: Do not freeze immutable objects, as freezing them has no effect.

@@ -7,6 +9,9 @@ module Request
# @attr_reader [String] base_url the base url of our api. (expected: http://example.com - without the trailing slash).
# @attr_reader [Hash] headers the headers being sent in the request.
class BaseRequest
TIMEOUT = 40.freeze

Choose a reason for hiding this comment

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

Style/RedundantFreeze: Do not freeze immutable objects, as freezing them has no effect.

end
end
end
end

Choose a reason for hiding this comment

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

Layout/TrailingBlankLines: 1 trailing blank lines detected.

if endpoint.query
# Returns [ ["key", "value"], ["key", "value"] ]
key_value_array = URI.decode_www_form(endpoint.query)
key_value_array.map! { |key_value_pair| [ key_value_pair[0].to_sym, key_value_pair[1] ] }

Choose a reason for hiding this comment

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

Layout/SpaceInsideArrayLiteralBrackets: Do not use space inside array brackets.

exception_class = Parliament::ClientError
when Net::HTTPServerError # 5xx Status
exception_class = Parliament::ServerError
def separate_uri(query_url, query_params, additional_params)

Choose a reason for hiding this comment

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

Metrics/MethodLength: Method has too many lines. [11/10]

(response.headers&.[]('Content-Length').nil? && response.body.empty?)
elsif /\A4\w{2}/.match(response.code.to_s) # 4xx Status
Parliament::ClientError
elsif /\A5\w{2}/.match(response.code.to_s) # 5xx Status

Choose a reason for hiding this comment

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

Performance/RedundantMatch: Use =~ in places where the MatchData returned by #match will not be used.

exception_class = if response.success? # 2xx Status
Parliament::NoContentResponseError if response.headers&.[]('Content-Length') == '0' ||
(response.headers&.[]('Content-Length').nil? && response.body.empty?)
elsif /\A4\w{2}/.match(response.code.to_s) # 4xx Status

Choose a reason for hiding this comment

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

Performance/RedundantMatch: Use =~ in places where the MatchData returned by #match will not be used.

headers.each do |key, value|
request.add_field(key, value)
end
def headers

Choose a reason for hiding this comment

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

Lint/DuplicateMethods: Method Parliament::Request::BaseRequest#headers is defined at both lib/parliament/request/base_request.rb:15 and lib/parliament/request/base_request.rb:183.

request = Net::HTTP::Post.new(
endpoint_uri.request_uri,
'Content-Type' => 'application/json'
def post(params: nil, body: nil, timeout: TIMEOUT, connecttimeout: CONNECTTIMEOUT)

Choose a reason for hiding this comment

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

Metrics/MethodLength: Method has too many lines. [15/10]

net_response = http.start do |h|
api_request = Net::HTTP::Get.new(endpoint_uri.request_uri)
add_headers(api_request)
def get(params: nil, timeout: TIMEOUT, connecttimeout: CONNECTTIMEOUT)

Choose a reason for hiding this comment

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

Metrics/MethodLength: Method has too many lines. [14/10]

@@ -7,6 +9,9 @@ module Request
# @attr_reader [String] base_url the base url of our api. (expected: http://example.com - without the trailing slash).
# @attr_reader [Hash] headers the headers being sent in the request.
class BaseRequest
TIMEOUT = 40.freeze
CONNECTTIMEOUT = 5.freeze

Choose a reason for hiding this comment

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

Style/RedundantFreeze: Do not freeze immutable objects, as freezing them has no effect.

@@ -7,6 +9,9 @@ module Request
# @attr_reader [String] base_url the base url of our api. (expected: http://example.com - without the trailing slash).
# @attr_reader [Hash] headers the headers being sent in the request.
class BaseRequest
TIMEOUT = 40.freeze

Choose a reason for hiding this comment

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

Style/RedundantFreeze: Do not freeze immutable objects, as freezing them has no effect.

@@ -13,7 +13,7 @@ class UrlRequest < Parliament::Request::BaseRequest
#
# @param [String] base_url the base url of our api. (expected: http://example.com - without the trailing slash).
# @param [Hash] headers the headers being sent in the request.
# @param [Parliament::Builder] builder the builder to use in order to build a response.
# @param [Parliament::Builder] builder the builder to use in order to build a response.{{}}
Copy link
Contributor

Choose a reason for hiding this comment

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

Curly braces needed?

@mattrayner mattrayner merged commit 048d47e into master Jul 9, 2018
@mattrayner mattrayner deleted the typheous-investigation branch July 9, 2018 12:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants