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

Allow using specific consitency on all methods #165

Closed
wants to merge 5 commits into from
Closed

Allow using specific consitency on all methods #165

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5b66143
Allow using specific consitency on all methods
pierresouchay Apr 16, 2018
7a0097b
Rubocop fixes
pierresouchay Apr 18, 2018
d77cc72
More rubocop fixes
pierresouchay Apr 18, 2018
91ab964
Updated documentation for all methods
pierresouchay May 2, 2018
8241bdb
Fixed unicode space in doc
pierresouchay May 9, 2018
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
2 changes: 1 addition & 1 deletion lib/diplomat/acl.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ def info(id, options = nil, not_found = :reject, found = :return)
@options = options
url = ["/v1/acl/info/#{id}"]
url << check_acl_token
url << use_consistency(options)
url << use_consistency(options) if use_consistency(options, nil)

raw = @conn_no_err.get concat_url url
if raw.status == 200 && raw.body.chomp != 'null'
Expand Down
4 changes: 0 additions & 4 deletions lib/diplomat/api_options.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,6 @@ def use_cas(options)
options ? use_named_parameter('cas', options[:cas]) : []
end

def use_consistency(options)
options && options[:consistency] ? [options[:consistency].to_s] : []
end

# Mapping for valid key/value store transaction verbs and required parameters
#
# @return [Hash] valid key/store transaction verbs and required parameters
Expand Down
3 changes: 2 additions & 1 deletion lib/diplomat/datacenter.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,9 @@ class Datacenter < Diplomat::RestClient
# Get an array of all avaliable datacenters accessible by the local consul agent
# @param meta [Hash] output structure containing header information about the request (index)
Copy link

Choose a reason for hiding this comment

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

Comment / Doc shall be updated to take into account the new param

# @return [OpenStruct] all datacenters avaliable to this consul agent
def get(meta = nil)
def get(meta = nil, options = nil)
url = ['/v1/catalog/datacenters']
url << use_consistency(options) if use_consistency(options, nil)
Copy link

Choose a reason for hiding this comment

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

why use 'if use_consistency' everywhere while it could be integrated into use_consistency (or a wrapper of both...) it's not very conservative if you forget one use_consistency call

Copy link
Member Author

Choose a reason for hiding this comment

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

Did it in next patch


ret = @conn.get concat_url url

Expand Down
4 changes: 4 additions & 0 deletions lib/diplomat/health.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ class Health < Diplomat::RestClient
def node(n, options = nil)
url = ["/v1/health/node/#{n}"]
url << use_named_parameter('dc', options[:dc]) if options && options[:dc]
url << use_consistency(options) if use_consistency(options, nil)

# If the request fails, it's probably due to a bad path
# so return a PathNotFound error.
Expand All @@ -27,6 +28,7 @@ def node(n, options = nil)
def checks(s, options = nil)
url = ["/v1/health/checks/#{s}"]
url << use_named_parameter('dc', options[:dc]) if options && options[:dc]
url << use_consistency(options) if use_consistency(options, nil)

# If the request fails, it's probably due to a bad path
# so return a PathNotFound error.
Expand All @@ -49,6 +51,7 @@ def service(s, options = nil)
url << 'passing' if options && options[:passing]
url << use_named_parameter('tag', options[:tag]) if options && options[:tag]
url << use_named_parameter('near', options[:near]) if options && options[:near]
url << use_consistency(options) if use_consistency(options, nil)

# If the request fails, it's probably due to a bad path
# so return a PathNotFound error.
Expand All @@ -68,6 +71,7 @@ def state(s, options = nil)
url = ["/v1/health/state/#{s}"]
url << use_named_parameter('dc', options[:dc]) if options && options[:dc]
url << use_named_parameter('near', options[:near]) if options && options[:near]
url << use_consistency(options) if use_consistency(options, nil)

# If the request fails, it's probably due to a bad path
# so return a PathNotFound error.
Expand Down
1 change: 1 addition & 0 deletions lib/diplomat/node.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ def get(key, options = nil)
url = ["/v1/catalog/node/#{key}"]
url += check_acl_token
url << use_named_parameter('dc', options[:dc]) if options && options[:dc]
url << use_consistency(options) if use_consistency(options, nil)

# If the request fails, it's probably due to a bad path
# so return a PathNotFound error.
Expand Down
4 changes: 3 additions & 1 deletion lib/diplomat/nodes.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,16 @@ class Nodes < Diplomat::RestClient
# Get all nodes
# @deprecated Please use Diplomat::Node instead.
# @return [OpenStruct] all data associated with the nodes in catalog
def get
def get(options = nil)
ret = @conn.get '/v1/catalog/nodes'
url << use_consistency(options) if use_consistency(options, nil)
JSON.parse(ret.body)
end

def get_all(options = nil)
url = ['/v1/catalog/nodes']
url << use_named_parameter('dc', options[:dc]) if options && options[:dc]
url << use_consistency(options) if use_consistency(options, nil)

ret = @conn.get concat_url url
JSON.parse(ret.body).map { |service| OpenStruct.new service }
Expand Down
3 changes: 2 additions & 1 deletion lib/diplomat/query.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ def get(key, options = nil)
url = ["/v1/query/#{key}"]
url += check_acl_token
url << use_named_parameter('dc', options[:dc]) if options && options[:dc]
url << use_consistency(options) if use_consistency(options, nil)

ret = @conn.get concat_url url
JSON.parse(ret.body).map { |query| OpenStruct.new query }
Expand All @@ -27,7 +28,7 @@ def get_all(options = nil)
url = ['/v1/query']
url += check_acl_token
url << use_named_parameter('dc', options[:dc]) if options && options[:dc]

url << use_consistency(options) if use_consistency(options, nil)
ret = @conn.get concat_url url
JSON.parse(ret.body).map { |query| OpenStruct.new query }
rescue Faraday::ClientError
Expand Down
13 changes: 13 additions & 0 deletions lib/diplomat/rest_client.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,19 @@ def use_named_parameter(name, value)
value ? ["#{name}=#{value}"] : []
end

def use_consistency(options, default_value = [])
Copy link

Choose a reason for hiding this comment

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

I don't get the point of having a default value if it's always set to nil, and checked if its value is not nil... Better just remove the if use_consistency(options, nil) and use the default value

Copy link
Member Author

Choose a reason for hiding this comment

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

It was to avoid modifying lib/diplomat/kv.rb, but I did the other way around finally in next patch

return default_value unless options
case options[:consistency]
when 'consistent'
return use_named_parameter('consistent', 'consistent')
when 'stale'
return use_named_parameter('stale', 'stale')
when 'leader'
return use_named_parameter('leader', 'leader')
end
default_value
end

# Assemble a url from an array of parts.
# @param parts [Array] the url chunks to be assembled
# @return [String] the resultant url string
Expand Down
2 changes: 2 additions & 0 deletions lib/diplomat/service.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ def get(key, scope = :first, options = nil, meta = nil)
url << use_named_parameter('index', options[:index]) if options && options[:index]
url << use_named_parameter('dc', options[:dc]) if options && options[:dc]
url << use_named_parameter('tag', options[:tag]) if options && options[:tag]
url << use_consistency(options) if use_consistency(options, nil)

# If the request fails, it's probably due to a bad path
# so return a PathNotFound error.
Expand Down Expand Up @@ -53,6 +54,7 @@ def get_all(options = nil)
url = ['/v1/catalog/services']
url += check_acl_token
url << use_named_parameter('dc', options[:dc]) if options && options[:dc]
url << use_consistency(options) if use_consistency(options, nil)
begin
ret = @conn.get concat_url url
rescue Faraday::ClientError
Expand Down