Support for flexible billing and usage records

[ob-stripe]

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

Since stripe-python uses keyword arguments for request parameters, I simply added subscription_item as a a required argument to UsageRecord.create().

[alexander-stripe]

Thanks! This looks good to me

[brandur-stripe]

Thanks for sorting this out Olivier!

I just wanted to flag that in most of our other languages, the subscription_item keyword has been part of the optional arguments, so this will break that pattern a little bit there (although in retrospect, making it a required parameter is probably better). I took a look through stripe-python to see if there might be other examples of a required argument parameter like this, but I don't think there are (ag "def create" didn't reveal any anyway).

I'm okay with it if you're comfortable though — what do you think?

[ob-stripe]

@brandur-stripe My reasoning was that if we wanted to do exactly the same thing that we did in stripe-ruby and stripe-php, we'd have to check if subscription_item was a key in params and then raise an exception.

For the user though, it makes little difference. In nearly all cases, they'll be passing keyword arguments, e.g. calling resource = stripe.UsageRecord.create(subscription_item='si_...', ...). If the subscription_item argument is missing, it makes little practical difference if the error that is returned is a TypeError returned by the interpreter because of the missing required argument, or an exception that we'd raise ourselves (because of the missing "required" argument).

I guess doing it this way does make it possible to pass subscription_item as a positional argument (e.g. stripe.UsageRecord.create('si_...', ...) but that's certainly not the intended usage. Python has a syntax for requiring a keyword argument (PEP-3102) but it's not available in Python 2 unfortunately.

I'm on the fence about all this, so if you think it's better, I can certainly switch to the manual exception raising pattern we've used in other libraries.

[brandur-stripe]

I guess doing it this way does make it possible to pass subscription_item as a positional argument (e.g. stripe.UsageRecord.create('si_...', ...) but that's certainly not the intended usage. Python has a syntax for requiring a keyword argument (PEP-3102) but it's not available in Python 2 unfortunately.

Ah, interesting. I didn't know about that.

It's kind of a similar story in Ruby — I actually got out of the habit of using required named parameters in stripe-ruby because 1.9 support was so long running. Now that I think about it though, we only support 2+, and could plausibly change UsageRecord.create there to be a required named parameter for better visibility/ergonomics.

Regarding Python though: what do you think about possibly making it an optional parameter that throws an error with the expectation that eventually we could change it to take advantage of PEP-3102? I know today the Python 2/3 divide seems long-standing and permanent, but the community seems to be pretty committed about 2020 being the year where from an official standpoint, Python 2 will be jettisoned into the sun. I'm still not sure that we'll be able to drop support for Python 2 then, but I think it would at least become a possibility.

I don't feel too strongly about either path though, so I'll leave it up to you.

[ob-stripe]

@brandur-stripe Works for me. I've updated the PR so that UsageRecord.create has the exact same signature as CreateableAPIResource.create, and manually checking if subscription_item is in the params dict. I've also added a test to check that we raise an exception if subscription_item is missing.

ptal @brandur-stripe

[brandur-stripe]

Amazing!

LGTM.