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

Wire an http.Client from NewBackends through to backends #627

Merged
merged 1 commit into from
Jul 17, 2018

Conversation

brandur
Copy link
Contributor

@brandur brandur commented Jul 17, 2018

In #611 I accidentally introduced a regression in the NewBackends
function in that an http.Client passed into the function isn't
actually wired through to the new backends.

Here we fix that problem by changing the invocations to use
GetBackendWithConfig like they were supposed to.

Fixes #626.

In #611 I accidentally introduced a regression in the `NewBackends`
function in that an `http.Client` passed into the function isn't
actually wired through to the new backends.

Here we fix that problem by changing the invocations to use
`GetBackendWithConfig` like they were supposed to.

Fixes #626.
httpClient := &http.Client{}
backends := stripe.NewBackends(httpClient)
assert.Equal(t, httpClient, backends.API.(*stripe.BackendConfiguration).HTTPClient)
assert.Equal(t, httpClient, backends.Uploads.(*stripe.BackendConfiguration).HTTPClient)
Copy link
Contributor

Choose a reason for hiding this comment

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

These tests aren't amazing because we have to cast to the private-ish BackendConfiguration. I may remove them in a future refactor, but for now they give us at least some confidence that this fix works correctly.

@brandur-stripe
Copy link
Contributor

r? @remi-stripe Thanks!

Copy link
Contributor

@remi-stripe remi-stripe left a comment

Choose a reason for hiding this comment

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

LGTM, thank you for the quick fix!

@brandur-stripe brandur-stripe merged commit 811b49e into master Jul 17, 2018
@brandur-stripe brandur-stripe deleted the brandur-fix-new-backends branch July 17, 2018 19:50
nadaismail-stripe pushed a commit that referenced this pull request Oct 18, 2024
* Bump sfdx

* MFA instructions

* Scratch setup settings to disable mfa on all ip ranges

* Misc notes from hacking
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The http.Client passed to NewBackends isn't being used
4 participants