Unify request/response handling

[rattrayalex-stripe]

r? @ob-stripe
cc @brandur-stripe
cc @stripe/api-libraries

Goal is to solve problems like #806 but also move ~everything to make requests the same way, with a single function call (except truly exceptional cases, where we need logic in handlers for backcompat etc).

Have updated this somewhat per feedback below.

[brandur-stripe]

Thanks for putting this together Alex!

So at first glance I have to say that this seemed a tad janky to me — something about switching this relatively important behavior off of object seems like it creates a bit of a subtle and potentially non-obvious semantic that users have to learn. That said, I think it should mostly work — accessing something like an issuing card's details is an apparent action on the object itself so it mutates, while doing something like source.source_transactions is obviously meant to retrieve a new set of objects.

Overall, my preference would be to leave things as they are because it's the most explicit/transparent, but I guess this would be okay if it's critical to client gen success.

Another note: we've discussed quite a few times in the past about how we'd eventually like to a "service" closer to what's seen in .NET or Go so that all your API calls would be on separate objects like ChargeService instead of on the objects directly, and which has the nice effect of decoupling data from API invocations. Getting to that would hopefully let us unwind this patch because all API calls would always return new objects.

Also curious to hear @ob-stripe's thoughts as well!

[rattrayalex-stripe]

I think the one case this is missing is on static methods, which should not call initialize_from, but yes, I think the object check is actually pretty much the Actually Correct way to do this, given the way Util.convert_to_stripe_object works - but if you can think of a more formally correct way to go, I'm certainly all ears.

Another thing I considered was checking for id equality, but that may not behavior correctly with singletons.

Overall, my preference would be to leave things as they are because it's the most explicit/transparent, but I guess this would be okay if it's critical to client gen success.

It's definitely not critical to client gen success – it's more of a personal preference in finding the existing way things are written a bit confusing / hard to follow – why are these things spelled out the exact same way every time? What's really going on? Why is it sometimes initialize_from and others Util.convert_to_stripe_objet? Formalizing these distinctions in one place feels much better to me.

tl;dr, as a reader to the library, I think I'd much prefer the same method in every place, with nice kwargs spelling out what the differences in behavior are.

[brandur-stripe]

Formalizing these distinctions in one place feels much better to me.

Yeah, fair enough. If we can get the same rule distributed everywhere, then I'm all for that.

What I was getting at is that moving the logic up a method likely means that you just have to go up one layer of abstraction to understand what's going to happen, but it's probably not that big of a deal.

[ob-stripe]

Sorry for the super late reply!

Generally agree with Brandur, this feels a bit janky to me but I won't block on it if it makes codegen easier.

Nit: all requests return Stripe objects, so I would just name the method make_request.