-
Notifications
You must be signed in to change notification settings - Fork 44
History row conflict concurrency issue #2203
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
Conversation
| if identifier: | ||
| return cls.query.filter_by(id=identifier).with_for_update().one_or_none() | ||
| query = cls.query.filter_by(id=identifier).with_for_update() | ||
| query = query.populate_existing() |
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.
If you have an existing fetch of eg payment_account.find_by_id, this will refresh the row, if you don't do this you may have old data which causes a row conflict, we didn't see this in the versioned unit test because it only grabbed the data once
| account_id=invoice.payment_account_id, | ||
| created_invoice_id=invoice.id, | ||
| ).flush() | ||
| ).save() |
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.
This should be save, because the credit is already created in CMS
I was running into weird issues with flush to do with deadlocks it would trigger a large query it seemed like, this should be save anyways
| case _: | ||
| # I don't believe there are CC (DirectPay flow not DirectSale) refunds, wouldn't want a credit back | ||
| raise NotImplementedError(f"Payment method {invoice.payment_method_code} not implemented for credits.") | ||
| payment_account.save() |
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.
This should be save, because the credit is already created in CMS
| "watchdog": { | ||
| "level": "WARNING", | ||
| "handlers": ["structured"], | ||
| "propagate": false |
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.
was getting weird spam calls
| pad_account_credit = payment_account.pad_credit or 0 | ||
| payment_account.pad_credit = 0 if pad_account_credit < invoice.total else pad_account_credit - invoice.total | ||
| payment_account.flush() | ||
| payment_account.save() |
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.
invoice creation commits after this anyways, should be ok
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of the sbc-pay license (Apache 2.0).