Skip to content

[16.0][BP][IMP] connector_extension: remove cr.commit() from bind_export#893

Closed
deeniiz wants to merge 2 commits into16.0from
16.0-fix-ref-connector_extension
Closed

[16.0][BP][IMP] connector_extension: remove cr.commit() from bind_export#893
deeniiz wants to merge 2 commits into16.0from
16.0-fix-ref-connector_extension

Conversation

@deeniiz
Copy link
Copy Markdown
Collaborator

@deeniiz deeniiz commented Apr 17, 2026

No description provided.

eantones and others added 2 commits April 17, 2026 09:46
In Odoo 18, queue_job forbids commits during job execution via
_prevent_commit to avoid releasing the job lock prematurely.
The cr.commit() in bind_export was an optimization for concurrent
binding creation that is no longer compatible. The _retry_unique_violation
context manager still handles race conditions via PostgreSQL's UNIQUE
constraint without needing an explicit commit.
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 17, 2026

Codecov Report

❌ Patch coverage is 20.00000% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 44.28%. Comparing base (20d034b) to head (8ddca89).

Files with missing lines Patch % Lines
connector_extension/components/binder.py 20.00% 4 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             16.0     #893   +/-   ##
=======================================
  Coverage   44.28%   44.28%           
=======================================
  Files         317      317           
  Lines        6294     6294           
  Branches     1000      999    -1     
=======================================
  Hits         2787     2787           
  Misses       3486     3486           
  Partials       21       21           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@deeniiz deeniiz requested a review from eantones April 17, 2026 10:45
@deeniiz deeniiz closed this Apr 24, 2026
@eantones
Copy link
Copy Markdown
Member

eantones commented Apr 28, 2026

Superseded by #895.

This PR's approach (removing cr.commit() from bind_export and relying on _retry_unique_violation) was rejected because it breaks the eager-commit anti-orphan guarantee: if the job fails after SAP has already created the external record, rollback loses the binding and the next retry creates a duplicate in SAP. _retry_unique_violation only protects against concurrent-job races (and even that protection is weak under PostgreSQL REPEATABLE READ since the unique constraint is on (backend_id, external_id), where external_id is assigned by SAP — duplicates in SAP can't be detected client-side).

Replaced by #895, which keeps the eager cr.commit() and instead uses the official OCA toggle allow_commit=True on the queue.job.function records (introduced by OCA/queue#910), with an additional cursor-rebind workaround at connector_extension/models/binding/binding.py for Job.in_temporary_env()'s args limitation (acknowledged by guewen in OCA/queue#889).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants