Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update stripe to latest package/api version #1345

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

wlcx
Copy link
Member

@wlcx wlcx commented Jan 29, 2024

Not suggesting that we necessarily merge this in time for ticket sales!

Issues:

  • Check payment/refund with a test stripe account
  • Why does mypy thing stripe_client is None|Any?

@wlcx wlcx force-pushed the lets-upgrade-stripe branch from 9d47aa4 to 510992d Compare January 29, 2024 22:14
@wlcx
Copy link
Member Author

wlcx commented Jan 29, 2024

Hang on, why not just instantiate the stripe client each time? Are we very concerned with performance? Less global state feels better...

@marksteward
Copy link
Member

It's more about connecting the config to the library in one place, rather than performance. You can replace it with a get_stripe_client function but is that really any better?

@wlcx
Copy link
Member Author

wlcx commented Jan 29, 2024

Yeah that's what I was thinking. In terms of type checking properly and avoiding globals, I'd have thought so!

@wlcx wlcx force-pushed the lets-upgrade-stripe branch from 510992d to 6f0afc4 Compare January 30, 2024 00:07
@russss russss marked this pull request as draft February 2, 2024 21:29
@wlcx wlcx force-pushed the lets-upgrade-stripe branch from ef89edb to 7e52407 Compare February 11, 2024 12:54
@wlcx wlcx marked this pull request as ready for review February 11, 2024 13:02
@wlcx wlcx changed the title [draft] Update stripe to latest package/api version Update stripe to latest package/api version Feb 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants