Add Get/SetHTTPClient to allow overriding the http.Client used for requests

[gtosh4]

This is useful for adding timeouts (see included example), rate limiting, or caching at the transport layer.

[gtosh4]

Forgot to check open PRs, but this change differs from #38 in that this puts the implementation of any retries or rate limits up to the user. My use-case is mostly around wanting to add a cache (since many of the game data APIs don't change much) which would be much easier through wrapping the Transport.

[FuzzyStatic]

Hey @gtosh4,

Thanks for your PR. I understand what you are trying to achieve, but my concern with this is having the HTTP behavior possibly change down the line when using a specific client. If you need different caching, timeout, and rate-limiting behaviors it would be better to make a different blizzard client for each scenario. Unfortunately, I did not consider this when I initially created this module.

For me, the best approach would be to overhaul the New function to take in a Config structure that specifies HTTP client parameters along with Blizzard values. This would allow for future extensibility for things like this and is very explicit what values are used for each client.

Does that make sense?

[gtosh4]

having the HTTP behavior possibly change down the line when using a specific client

Not sure I understand, can you clarify what concern you have here? That this library will change the client, and users behaviour would not "keep up" if they overwrite the http client? Besides for the oauth2 stuff, what cases do you expect this to occur or is this preemptive?

If you need different caching, timeout, and rate-limiting behaviors it would be better to make a different blizzard client for each scenario

By "make a different blizzard client", do you mean an alternative library or do you mean different instances of blizzard.Client? The approach in the PR would enable doing all 3 of those behaviours as-needed without any additional code in the library (by letting the users control that behaviour). 

the best approach would be to overhaul the New function to take in a Config structure that specifies HTTP client parameters along with Blizzard values

I agree, that a batteries-included, best practices, clean API would be ideal and I hope that #38 can continue to get traction (seems like it's stalled however). In lieu of these features, the approach in this PR enables customization now, without waiting for those features to be implemented.

Feel free to close if you disagree with the approach, I wouldn't mind. I can continue to use my fork so I'm not blocked.

[FuzzyStatic]

@gtosh4 I've created a v3 version that allows for a user to add their own HTTP client inline during Blizzard client creation. I feel this makes more sense.

Let me know your thoughts.

[gtosh4]

v3 works for me 👍

Closing this