[16.0][FIX] connector_sapb1: enable allow_commit on export_record job funct…#895
[16.0][FIX] connector_sapb1: enable allow_commit on export_record job funct…#895
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 16.0 #895 +/- ##
==========================================
- Coverage 44.28% 43.75% -0.53%
==========================================
Files 317 325 +8
Lines 6294 6470 +176
Branches 1000 1011 +11
==========================================
+ Hits 2787 2831 +44
- Misses 3486 3618 +132
Partials 21 21 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…ions bind_export in connector_extension performs cr.commit() to persist the binding immediately after the SAP HTTP PATCH succeeds. This is intentional: it prevents duplicate addresses being created in SAP on job retry (when the adapter's -2035 counter would increment AddressName to "(2)", "(3)" etc. for the same partner on each retry attempt). Since queue_job 16.0.2.13.2 (OCA/queue#892), the _prevent_commit guard raises RuntimeError on any cr.commit() inside a job. Setting allow_commit =True on the Job Function makes perform() run in a temporary env with a separate DB cursor: the commit is real but does not release the main cursor's queue_job row-lock, so there is no double execution risk. Post-migration updates the existing auto-created queue.job.function records for: - sapb1.binding - sapb1.res.partner - sapb1.sale.order - sapb1.sale.order.line - sapb1.product.product Refs: https://github.com/OCA/queue/wiki/Upgrade-warning:-commits-inside-jobs Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
deeniiz
force-pushed
the
16.0-fix-connector_sapb1-allow_commit_export_record
branch
from
054ecba to
22d2ae3
Compare
…in export entry-points When a queue.job.function has allow_commit=True, queue_job 16.0's Job.in_temporary_env() opens a new DB cursor and switches self.recordset.env to it. However, args passed to the job method (backend_record, relation) keep the outer cursor — they were deserialized from the queue.job's args field with the runner's env, and queue_job does NOT re-bind them to the temporary env. When binding.export_record(backend_record, relation) calls backend_record.work_on(self._name), the resulting WorkContext.env returns self.collection.env (component/core.py:271-277) — i.e. the OUTER cursor that has the _prevent_commit monkey-patch applied. binder.bind_export() then commits via self.env.cr.commit() and hits the forbidden_commit guard, so allow_commit=True alone does NOT take effect for export entry-points whose flow ends in bind_export. Fix: rebind backend_record (and relation) to a new env that copies the arg's ORIGINAL env (preserving uid, su, context) and only swaps cr to self.env.cr — the temporary cursor opened by Job.in_temporary_env(). This is the same pattern queue_job itself uses internally (env(cr=new_cr)) and is minimally invasive: it only changes the cursor, leaving the user/su/context that were captured at enqueue time intact. Idempotent when self.env.cr already equals the arg's cr (calls outside of a queue job). Scope: only export entry-points need this. Imports do not commit — bind_import does not call cr.commit() (was removed in 22c9fb2) — so they never hit the forbidden_commit guard and don't need allow_commit=True nor the cursor propagation. Verified against Novolux production task #2539: queue.job 9434462 (<sapb1.sale.order>.export_record, allow_commit=true) was still failing with RuntimeError("Commit is forbidden in queue jobs") at binder.py:315 because backend_record arrived on the outer cursor.
|
Supersedes #893. #893 removed This PR keeps the eager commit at The latest commit Verified in production after first deployment of this PR's earlier commits: queue.job 9434462 ( |
|
Hi @eantones
Fyi I was not aware of the issue with arguments using another env that need to be rebound to the inner env. This may be something to be adressed in queue job, feel free to report |
|
Hi @guewen, thanks for the reply — much appreciated. We'll open an issue on OCA/queue and link it back here. |
3 participants
…ions
bind_export in connector_extension performs cr.commit() to persist the binding immediately after the SAP HTTP PATCH succeeds. This is intentional: it prevents duplicate addresses being created in SAP on job retry (when the adapter's -2035 counter would increment AddressName to "(2)", "(3)" etc. for the same partner on each retry attempt).
Since queue_job 16.0.2.13.2 (OCA/queue#892), the _prevent_commit guard raises RuntimeError on any cr.commit() inside a job. Setting allow_commit =True on the Job Function makes perform() run in a temporary env with a separate DB cursor: the commit is real but does not release the main cursor's queue_job row-lock, so there is no double execution risk.
Post-migration updates the existing auto-created queue.job.function records for:
Refs: https://github.com/OCA/queue/wiki/Upgrade-warning:-commits-inside-jobs