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
Remove redundant transaction in customers.create #341
Remove redundant transaction in customers.create #341
Conversation
Some context for this one, @paltman. I've been getting issues when using customer.create whereby the webhook It seems unlikely that the create Customer query would be uncommitted for a long enough period due transaction.atomic() such that the webhook arrives first, and yet I'm seeing this occur a few times per day. Any thoughts? I'd rather not preemptively create customers with Stripe. |
I'm trying to recall why I even had the transaction there in the first place. |
I'm good to merge this. Can you fix the lint issue first though, @lukeburden? |
fb5977f
to
7c486a1
Compare
So, I hit the reason why you added the transaction in the first place. Nasty, one that I've also hit in various projects. I've done some minor refactoring of the |
7c486a1
to
680a01f
Compare
Thanks! |
if plan and charge_immediately: | ||
invoices.create_and_pay(cus) | ||
else: | ||
# remove this extra customer as it is not needed | ||
stripe.Customer.retrieve(stripe_customer["id"]).delete() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't it be better to create it only in case there is no local customer already in the first pace?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that would make sense. Go for it!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool, see #364.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just for reference: finally rejected in #364.
The use of transaction.atomic() wrapping the Customer.objects.create call is redundant as there is only a single query which will either succeed or fail. If a customer already exists, an IntegrityError will still be raised.