Skip to content

Add configuration overrides to Mirren::Client#6

Merged
brianvh merged 4 commits intomasterfrom
bh_add_client_config
Apr 15, 2020
Merged

Add configuration overrides to Mirren::Client#6
brianvh merged 4 commits intomasterfrom
bh_add_client_config

Conversation

@brianvh
Copy link
Collaborator

@brianvh brianvh commented Apr 15, 2020

This provides an easy way to override the default ENV values used to configure the consumer's access to the MiningRigRentals API. This required pulling all references to ENV values, down in the Api module classes, up into the Client. I modified the Api::Auth class to be a sub-configuration object on the Client, that the Api request methods pass down to the Request class.
I also refactored the Request class to make it easier to pass values around, without relying too heavily on instance variables and attr_reader.

@rschifflin

brianvh added 4 commits April 15, 2020 13:18
Add mirren/errors.rb and mirren/result_ext.rb.
Modify Auth to take :api_key and :api_secret as initialization values.
Modify Auth#call to take nonce and path arguments.
Modify Header to a simple "function" class, that takes auth and path
values and returns the correctly formatted header hash.
Modify .new to take host and auth, in addition to method and request.
Add :host, :auth, and :response to attr_reader and remove :path,
:get_params, and :post_params.
Modify call to make :path a positional argument.
Replace #call's if-else with an early return on 'success'.
Remove #request_args and place its return value inline in #response.
Modify #response to take path, get_params, and post_params as arguments.
Modify Mirren::Api methods to reflect changes to Request method
signatures.
Allows overriding :host, :api_key, :api_secret values, which default to
namespaced ENV values.
Modify MockClient Spec support class to reflect the changes to Client.
Modify Client integration spec to use new configuration override.
def initialize(opts = {})
@host = opts.fetch(:host, ENV['MIRREN_HOST'])
api_key = opts.fetch(:api_key, ENV['MIRREN_API_KEY'])
api_secret = opts.fetch(:api_secret, ENV['MIRREN_API_SECRET'])
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍 exactly what I hoped for


def stub_auth
Struct.new(:api_key) do
def call(*)
Copy link
Collaborator

@rschifflin rschifflin Apr 15, 2020

Choose a reason for hiding this comment

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

what is the * arg syntax? anonymous vararg? Does ** exist as anonymous keyword args?

Copy link
Collaborator Author

@brianvh brianvh Apr 15, 2020

Choose a reason for hiding this comment

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

It's a shorthand way of saying: accept any number and/or format of arguments to this method. It's usually paired with a simple call to super inside the method, which automatically collects all of the arguments and passes them to super.

I think an argument could be made that its use, here, is superfluous. I think I added it out of habit, as a way to ensure that the stub_auth Struct instance doesn't care what is passed to its #call method.

Copy link
Collaborator

Choose a reason for hiding this comment

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

makes sense, neat

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Also, the use of **opts, and similar, in a method call signature, is a way to tell Ruby to coerce the remaining passed in arguments as a hash. It will throw an exception if it can't coerce them. Use of **opts must be the last argument (not counting the implied block argument) specified on the method.

@rschifflin
Copy link
Collaborator

Awesome, very straightforward changes. LGTM 👍

@brianvh brianvh merged commit 8524eef into master Apr 15, 2020
@brianvh brianvh deleted the bh_add_client_config branch April 15, 2020 20:55
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.

2 participants