Enable configuration override on each consul api call #179
Conversation
tionebsalocin
force-pushed
the
options-everywhere
branch
4 times, most recently
from
252d422 to
372b832
Compare
lib/diplomat/datacenter.rb
Outdated
| def get(meta = nil) | ||
| url = ['/v1/catalog/datacenters'] | ||
| # rubocop:disable AbcSize, CyclomaticComplexity | ||
| def get(options = nil) |
There was a problem hiding this comment.
This is a breakage of API for those using meta to get information about request.
There was a problem hiding this comment.
Yes indeed. I've fixed this issue.
tionebsalocin
force-pushed
the
options-everywhere
branch
from
372b832 to
85628b4
Compare
There was a problem hiding this comment.
Looks good to me.
@tionebsalocin Would it be possible to update a bit documentation in another PR to reflect those changes?
|
LGTM. FAR the options / cutom_params parameters in send_{get,put,..}_request could be combined to avoid having too many ways to pass options. |
lib/diplomat/rest_client.rb
Outdated
|
|
||
| headers = { 'X-Consul-Token': configuration.acl_token } if configuration.acl_token | ||
|
|
||
| unless options.nil? |
There was a problem hiding this comment.
if options was a empty hash (by default) instead of nil, you could remove that condition
There was a problem hiding this comment.
I've set the default as empty hash and removed plenty of corresponding tests
lib/diplomat/rest_client.rb
Outdated
| req.body = JSON.dump(data) unless data.nil? | ||
| end | ||
| end | ||
| # rubocop:enable MethodLength, AbcSize, CyclomaticComplexity |
There was a problem hiding this comment.
what is the point of disabling/enabling those metrics. I would suggest to:
- disable cyclomatic complexity (and abcsize) measurement for which threshold are highly subjective
- really define what should be the max method length and stick to it.
There was a problem hiding this comment.
@tionebsalocin yes, sounds reasonable
There was a problem hiding this comment.
I've set default realistic values for these
| # rubocop:enable MethodLength, AbcSize, CyclomaticComplexity | ||
|
|
||
| # rubocop:disable MethodLength, AbcSize, CyclomaticComplexity | ||
| def send_put_request(connection, url, options, data, custom_params = nil) |
There was a problem hiding this comment.
OPT: using named arguments would make calls to this method easier to read
There was a problem hiding this comment.
I'll plan this for another review
| Diplomat.configuration.acl_token = nil | ||
| ev = Diplomat::Event.new(faraday) |
There was a problem hiding this comment.
great change, it makes test easier to read!
spec/lock_spec.rb
Outdated
| @@ -1,6 +1,7 @@ | |||
| require 'spec_helper' | |||
| require 'json' | |||
| require 'base64' | |||
| require 'webmock/rspec' | |||
There was a problem hiding this comment.
at this point, you could put all those requires in spec/spec_helper.rb and require it instead
tionebsalocin
force-pushed
the
options-everywhere
branch
from
85628b4 to
82ecacc
Compare
Using the configs hash target host or token can be different at each call
tionebsalocin
force-pushed
the
options-everywhere
branch
from
82ecacc to
35c369f
Compare
This release cleanup a lot the existing APIs while preserving ascending compatibility. It will avoid relying on side effects to configure diplomat and allow to override most configuration options per API call as implemented in #179. It is now easy to use one instance of the lib and to perform several calls with different tokens and/or consistency options for instance.
This release cleanup a lot the existing APIs while preserving ascending compatibility. It will avoid relying on side effects to configure diplomat and allow to override most configuration options per API call as implemented in #179. It is now easy to use one instance of the lib and to perform several calls with different tokens and/or consistency options for instance.
|
@scalp42 did which version did you use? (there were a few fixes after version 2.1.0...) |
Using the configs hash target host or token can be different at each call
e.g
value = Diplomat::Kv.get('key/path', token: '000-111-222-333-444', http_addr: 'http://consu01:8500', dc: 'dc1', stale: true)
Diplomat::Kv.put('key/path', value, token: '111-222-333-444-555', http_addr: 'http://consu02:8500', dc: 'dc2', stale: true)