Simplify version override

[sandstrom]

How about something like this?

Stripe.api_version = '2014-08-04'

Stripe.with_version('2019-10-17') do
  Stripe::SetupIntent.create({ usage: 'off_session' })
  # would use 2019-10-17 regardless of Stripe.api_version value
end

It's very easy to implement in Ruby with yield and ensure. Useful to avoid doing per-method override for a ton of methods.

[ob-stripe]

Hi @sandstrom. I think in the past we declined similar feature requests because we don't want to have too many different ways of accomplishing the same thing. E.g. in this case, setting the API version can be done globally (via Stripe.api_version=) or per request (by setting api_version in the opts argument) and I'm not sure that we want to support a third way, as it might not be worth the added maintenance burden and potential user confusion even if it could be occasionally useful.

cc @brandur-stripe @remi-stripe for thoughts/opinions

[remi-stripe]

Yeah, I think the opts argument is cleaner here because it's easier to reason with and pass around than having the whole block itself.

There's also a world in which we have custom clients you could create and reuse which we've been looking at. I'll let @ob-stripe provide a bit more details!

[ob-stripe]

There's also a world in which we have custom clients you could create and reuse which we've been looking at. I'll let @ob-stripe provide a bit more details!

Right, it's still very hypothetical, but I'm imagining something like this:

# create a client with no forced API version
client = Stripe::StripeClient.new('sk_test_123')
si = client.setup_intents.create(params)

# create a different client with a forced API version
client_with_api_version = Stripe::StripeClient.new('sk_test_123', api_version: '2019-10-17')
si2 = client_with_api_version.setup_intents.create(params)

Basically, we'd stop relying on the global configuration so much and hold all the configuration values in a StripeClient instance, which would make it easy to create different clients with different configuration values. (We would still probably have a "global client" for backwards compatibility.)

As said above, this is still very much theoretical at this point, but if you have any thoughts or feedback on this approach I'd love to hear it!

[sandstrom]

@ob-stripe Yes, client instances would be better than what I suggested above (though it's a larger change than what I suggested).

Although I'd say your ruby client is pretty high quality already, it's definitely more "standard" to use client instances instead of relying on global config. So that would definitely be an improvement! 🏅

Feel free to close this issue now, or keep it open and close later when the (hypothetical) client instances have landed.

Thanks for your replies & have a great weekend! 🌻

[brandur-stripe]

I'd be very much +1 to OB's proposal of supporting an api_version on StripeClient instances (and other things as well!) — it's a clean implementation, and I think very intuitive relative to how people might expect such a client instance to work.

[joeltaylor]

@brandonl-stripe @ob-stripe is this still inline with current direction the gem is headed? If so, I'd be happy to spike on a PR.

[brandur-stripe]

@joeltaylor Yep! (The latter part about the client, not so much the block pitched in the original comment.)

Feel free to take a shot at it, but also, I owe OB a prototype implementation myself now that we've made progress in most other languages, so I will hopefully get around to it in the next few weeks.

[joeltaylor]

@brandur-stripe I've been playing around with a few different routes and settled on leveraging the ability to pass a client instance to Stripe::ApiOperations::Request::ClassMethods.request.

Figured I'd share a brief example in the event it clashes with what you had in mind:

1. Dynamically define the resource methods within StripeClient to provide methods like Stripe::StripeClient#accounts. I figure this can be done via a configuration file to map resource names to methods in order to avoid loading all of the resources into memory.

The generated methods would essentially look something like:

def accounts
  ClientProxy.new(client: self, resource: Stripe::Account)
end

ClientProxy would essentially be a delegator that also passes the client instance.

    class ClientProxy
      def initialize(client:, resource:)
        @client = client
        @resource = resource
      end

      def method_missing(method, *args, &block)
        @resource.send(method, *args << { client: @client })
      end
    end

2. Update Stripe::StripeClient#execute_request to account for instance configuration with a fallback on global.

If this doesn't raise any red flags, I'm more than happy to flesh it out and open a PR. I managed to get a naive implementation working using the above snippets.

[brandur-stripe]

@joeltaylor Thanks for the continued interest on this! Just for perfect clarity, the eventual usage would look something like this, right?

client = Stripe::StripeClient.new
client.accounts.retrieve('acct_123')

If so, that's pretty much exactly what we'd expected it to look like.

[joeltaylor]

@brandonl-stripe Happy to help! Yeah, that's the intended usage that would be achieved with the approach I outlined.

I'll go ahead and hammer out a PR for further review. Thanks!

[brandur-stripe]

@joeltaylor Amazing!! Thank you.