Add support for /v1/topups endpoints #396
Conversation
|
This branch is stacked on #395. I'll do the rebase when that lands, but want to get these changes out for reviews. |
| from stripe.api_resources.abstract import UpdateableAPIResource | ||
| from stripe.api_resources.abstract import ListableAPIResource | ||
|
|
||
|
|
There was a problem hiding this comment.
This is the style used in other files, so I copied. I'll leave it as is to maintain consistency.
There was a problem hiding this comment.
It seems a little weird coming from other languages, but believe it or not, this is conformant with the most universally accepted style for Python, PEP 8. From the spec:
Surround top-level function and class definitions with two blank lines.
There's a check that will fail the test suite if the style is off, so you're safe from pulling something in that's wrong, but I'd suggest as a general guideline just doing what Jamu did and copying the style seen elsewhere.
stripe/api_resources/topup.py
Outdated
| @@ -0,0 +1,9 @@ | |||
| from __future__ import absolute_import, division, print_function | |||
There was a problem hiding this comment.
Are these imports used anywhere?
There was a problem hiding this comment.
Apparently not. I removed them.
tests/api_resources/test_topup.py
Outdated
| @@ -0,0 +1,60 @@ | |||
| from __future__ import absolute_import, division, print_function | |||
There was a problem hiding this comment.
Apparently not. I removed them.
|
|
||
| TEST_RESOURCE_ID = 'tu_123' | ||
|
|
||
|
|
There was a problem hiding this comment.
(Similarly to the comment above, I believe PEP 8 will require this doubled up blank line too.)
| 'post', | ||
| '/v1/topups/%s' % TEST_RESOURCE_ID | ||
| ) | ||
| self.assertIsInstance(resource, stripe.Topup) |
There was a problem hiding this comment.
Worth checking the resource has its metadata set to {'key': 'value'}?
There was a problem hiding this comment.
I tried this, but it looks like stripe-mock doesn't actually mutate received data correctly. I believe it just validates what it received and returns stock data.
There was a problem hiding this comment.
Yes that's correct Jamu. We want to fix this at some point, but for now it's not working checking any of the data that you get back.
|
Maybe just ack that we're good to go here as well, and we'll pull it in. Thanks! |
jkakar-stripe
force-pushed
the
jkakar/topup-client
branch
from
db30aaa to
912f505
Compare
|
lgtm |
|
r? @stripe/api-libraries |
|
LGTM. |
|
Released as 1.78.0. |
This add standard retrieve, create and update client support for the new
/v1/topupsendpoint.r? @apakulov-stripe @ccontinanza-stripe @chellman-stripe @kenneth-stripe @miguel-stripe
r? @stripe/api-libraries