Allow stripe_account to be set globally:

[Edouard-chin]

• When performing requests on the behalf of a managed account, stripe_account option must be passed everytime, this can become redundant
• Allowing to set the stripe_account globally makes thing easier for wrapping every request in a single method, the same way as it is when defining the api_key globally

@matthelm

[brandur]

Thanks!

So my only fear here is that other global fields like api_key and api_version are the types of information that you're going to set once and will almost certainly last throughout the entire lifecycle of your program. I would say that this isn't necessarily the case of stripe_account; although you may want to work under the context of a single account for some time, it's possible (and maybe even likely) that you might want to move to another context later on, which introduces questions around thread safety across the boundaries of mutations in this global state.

Can I ask you what your use case is for you is here?

[Edouard-chin]

Thanks for quick answer
We are switching from standalone account to managed account, we have many different places in our codebase where we retrieve Stripe object using API authentication via account specific api key.
Every call to Stripe are wrapped around a single method process_with_api_key that takes care of setting the desired api_key/version before yielding.

Moving to managed account would mean that we need to pass the stripe_account option in every method call to Stripe API and any future call would require so as well.

Instead we would like to set the stripe_account option (like we do for the version) inside our method wrapper so that we don't need to maintain multiple code paths as well as not have to remember in the future to pass the stripe_account option in the call.

[brandur]

Hey @Edouard-chin, so execute_request_with_rescues is at least to some degree what I'd consider a private-level method, and it would be nice to avoid referencing it in tests if possible.

test_helper defines a standardized way of mocking out HTTP requests; would you mind changing this check over to use that? Here is an example of a mock that would be very close to the one that you want to build.

[brandur]

@Edouard-chin Cool! I'm okay with bringing this in given that we currently don't give you a better way of doing this (we'll continue to work on that part of it).

I left one piece of feedback above. Would you mind taking a look at it, and then we can get this shipped. Thanks!

[Edouard-chin]

Hey @brandur , awesome this is going to simplify our life greatly! I modified the test as you advised to mock the response with the standardized way. However and maybe I was doing this wrong but the test was focusing on checking what headers were passed to the request.

This is no more the case as the post method arguments from the mock doesn't include the headers.

I'm fine if this is ok to you, I will have to rename the test and squash 😄

[brandur]

Could you also rename this one to "makes requests with the Stripe-Account header"?

[brandur]

This is no more the case as the post method arguments from the mock doesn't include the headers.

@Edouard-chin Doh! Okay, sorry that was my mistake.

We definitely want to test for the header though. Could you revert the last change and keep it the same except see if you can stub on the execute_request method instead? I think that this approach is slightly better because as you've seen, we already stub that elsewhere in the tests anyway.

Thanks! And sorry about the churn.

(And yes, please squash this all up if you can.)

[Edouard-chin]

@brandur No need to apologize thanks for your precious help !

All your comments should now be addressed, squashed commits and 💚 travis.

[brandur]

Thank-you :)

Released as 1.41.0.