[FIX] account_peppol_backport: use savepoint to handle SQL errors #25
Conversation
|
cc/ @jbaudoux |
wrap move creation in a savepoint to avoid leaving the cursor in an error state when an SQL constraint is raised, without this, the explicit commit after the exception fails fixes: #24
sbejaoui
force-pushed
the
16.0-issue_24-sbj
branch
from
a2d9e2b to
bbd089d
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 16.0 #25 +/- ##
==========================================
+ Coverage 65.49% 65.89% +0.39%
==========================================
Files 43 43
Lines 1339 1340 +1
Branches 188 188
==========================================
+ Hits 877 883 +6
+ Misses 405 402 -3
+ Partials 57 55 -2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Thanks! 👍 |
| proxy_acks.append(uuid) | ||
|
|
||
| if not tools.config['test_enable']: | ||
| self.env.cr.commit() # pylint: disable=invalid-commit |
There was a problem hiding this comment.
@sbidoul I remember you once mentioned that using explicit commits isn’t compatible with running crons through queue_job
We have a case that seems related: the “retrieve new document” cron is configured to run as a queue job. We noticed that some invoices gets created multiple times
When we looked at the data/timestamps, most duplicates were created before the job actually started, and only one invoice was created after it. However, the job itself was executed twice, so our hypothesis is:
- first run: Odoo downloaded the document, created the invoice, then commited
- then the run failed/crashed before before marked the document as already imported in server side
- second run: it downloaded the same document again
Do you think switching this cron back to “run in odoo” (not queue job) would solve it?
If not, do you have thoughts on this?
cc/ @phschmidt , @jbaudoux
There was a problem hiding this comment.
I remember you once mentioned that using explicit commits isn’t compatible with running crons through queue_job
Yes. When you commit in a queue_job, it releases the queue_job_lock and the dead job requeuer starts it again a few seconds later.
I'm planning to do a queue_job PR to raise an error on commit in queue jobs, but have not found the time to do it yet.
Do you think switching this cron back to “run in odoo” (not queue job) would solve it?
Very possibly yes. That said, the other rule of queue jobs (and crons) is that they should be idempotents. I dont know if this particular cron is.
wrap move creation in a savepoint to avoid leaving the cursor in an error state when an SQL constraint is raised, without this, the explicit commit after the exception fails
fixes: #24